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: <5d82e564-86bf-c26b-077a-d0bc14e2d3c3@quicinc.com>
Date:   Thu, 8 Sep 2022 23:23:56 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH 10/27] wifi: mac80211: isolate driver from inactive links
Content-Language: en-US
To:     Johannes Berg <johannes@sipsolutions.net>,
        <linux-wireless@vger.kernel.org>
CC:     Johannes Berg <johannes.berg@intel.com>,
        <ath11k@lists.infradead.org>
References: <20220902141259.377789-1-johannes@sipsolutions.net>
 <20220902161143.5ce3dad3be7c.I92e9f7a6c120cd4a3631baf486ad8b6aafcd796f@changeid>
From:   Wen Gong <quic_wgong@quicinc.com>
In-Reply-To: <20220902161143.5ce3dad3be7c.I92e9f7a6c120cd4a3631baf486ad8b6aafcd796f@changeid>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/2/2022 10:12 PM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> In order to let the driver select active links and properly
> make multi-link connections, as a first step isolate the
> driver from inactive links, and set the active links to be
> only the association link for client-side interfaces. For
> AP side nothing changes since APs always have to have all
> their links active.
>
> To simplify things, update the for_each_sta_active_link()
> API to include the appropriate vif pointer.
>
> This also implies not allocating a chanctx for an inactive
> link, which requires a few more changes.
>
> Since we now no longer try to program multiple links to the
> driver, remove the check in the MLME code.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   include/net/mac80211.h    |  30 +++----
>   net/mac80211/chan.c       |   6 ++
>   net/mac80211/driver-ops.c | 172 ++++++++++++++++++++++++++++++++++++++
>   net/mac80211/driver-ops.h | 165 ++++++------------------------------
>   net/mac80211/key.c        |   8 ++
>   net/mac80211/link.c       |  66 ++++++++++++---
>   net/mac80211/mlme.c       |  25 ++----
>   net/mac80211/util.c       |   2 +-
>   8 files changed, 286 insertions(+), 188 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d4e1d73d88cc..20a2f25a38fa 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1799,6 +1799,9 @@ struct ieee80211_vif_cfg {
>    * @link_conf: in case of MLD, the per-link BSS configuration,
>    *	indexed by link ID
>    * @valid_links: bitmap of valid links, or 0 for non-MLO.
> + * @active_links: The bitmap of active links, or 0 for non-MLO.
> + *	The driver shouldn't change this directly, but use the
> + *	API calls meant for that purpose.
>    * @addr: address of this interface
>    * @p2p: indicates whether this AP or STA interface is a p2p
>    *	interface, i.e. a GO or p2p-sta respectively
> @@ -1834,7 +1837,7 @@ struct ieee80211_vif {
>   	struct ieee80211_vif_cfg cfg;
>   	struct ieee80211_bss_conf bss_conf;
>   	struct ieee80211_bss_conf __rcu *link_conf[IEEE80211_MLD_MAX_NUM_LINKS];
> -	u16 valid_links;
> +	u16 valid_links, active_links;
>   	u8 addr[ETH_ALEN] __aligned(2);
>   	bool p2p;
>   
...
> @@ -123,11 +132,38 @@ static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
>   	return 0;
>   }
>   
> +static void ieee80211_set_vif_links_bitmaps(struct ieee80211_sub_if_data *sdata,
> +					    u16 links)
> +{
> +	sdata->vif.valid_links = links;
> +
> +	if (!links) {
> +		sdata->vif.active_links = 0;
> +		return;
> +	}
> +
> +	switch (sdata->vif.type) {
> +	case NL80211_IFTYPE_AP:
> +		/* in an AP all links are always active */
> +		sdata->vif.active_links = links;
> +		break;
> +	case NL80211_IFTYPE_STATION:
> +		if (sdata->vif.active_links)
> +			break;
> +		WARN_ON(hweight16(links) > 1);
> +		sdata->vif.active_links = links;
> +		break;
> +	default:
> +		WARN_ON(1);
> +	}
> +}
> +
Now I found it only active the primay link(the link for 
authentication/assoc request) in my station MLO test,
change_vif_links of struct ieee80211_ops *ops of driver will only be 
called one time for the primary link.
it means only one link for MLO.
I plan to revert this patch in my local test now.

Will you implement muti-links later?
> ...