Received-SPF: pass (google.com: domain of linux-wireless+bounces-8837-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <7869b9b29b6796c95fd5af649e4bd6696e56dcaf.camel@sipsolutions.net>
Subject: Re: [REGRESSION][BISECTED] wifi: RTL8821CE does not work in monitor
 mode
From: Johannes Berg <johannes@sipsolutions.net>
To: Ping-Ke Shih <pkshih@realtek.com>, Thorsten Leemhuis
	 <regressions@leemhuis.info>, Savyasaachi Vanga <savyasaachiv@gmail.com>, 
 Christian Heusel <christian@heusel.eu>
Cc: Kalle Valo <kvalo@kernel.org>, "linux-wireless@vger.kernel.org"
	 <linux-wireless@vger.kernel.org>, "regressions@lists.linux.dev"
	 <regressions@lists.linux.dev>
Date: Wed, 12 Jun 2024 09:58:12 +0200
In-Reply-To: <6a88e0d3e47ebbd1f0f383094e4804a627c41870.camel@sipsolutions.net>
References: 
	<chwoymvpzwtbmzryrlitpwmta5j6mtndocxsyqvdyikqu63lon@gfds653hkknl>
	 <a51f223f-18ac-4d67-9120-8da1c169b7eb@leemhuis.info>
	 <809d2332-625e-4a46-a77e-ca2e49b0d651@leemhuis.info>
	 <0e65ca6b471b4186a370b9a57de11abe@realtek.com>
	 <36c1f49acfcc2cc16c7e54ffeb76f1e93cb96e91.camel@sipsolutions.net>
	 <5318640d6eb74301b1fbf6d9385ba69e@realtek.com>
	 <6a88e0d3e47ebbd1f0f383094e4804a627c41870.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.2 (3.52.2-1.fc40) 
Precedence: bulk
MIME-Version: 1.0

On Wed, 2024-06-12 at 09:07 +0200, Johannes Berg wrote:
> On Wed, 2024-06-12 at 00:56 +0000, Ping-Ke Shih wrote:
>=20
>=20
> > > Just got pinged (sp?) about this, can you share the driver fix so I c=
an
> > > take a look what the issue is about?
> > >=20
> >=20
> > Please reference patch below. I copy this idea from rtw89 [1], which th=
e main
> > stuff is to add WANT_MONITOR_VIF and case NL80211_IFTYPE_MONITOR in add=
_interface().=20
>=20
> Ah, OK, but that gives me a hint. Yes, I see the issue now.
>=20
> OK it's not trivial and it might leave ath12k still not working (though
> not sure it ever did before? or maybe I'm missing something...), but I
> think I can fix this. Let's see.
>=20

I don't have any of the affected hardware, could someone test this?

https://p.sipsolutions.net/619a4ce4a197b2b4.txt

diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index dce37ba8ebe3..254d745832cb 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -311,6 +311,18 @@ int drv_assign_vif_chanctx(struct ieee80211_local *loc=
al,
 	might_sleep();
 	lockdep_assert_wiphy(local->hw.wiphy);
=20
+	/*
+	 * We should perhaps push emulate chanctx down and only
+	 * make it call ->config() when the chanctx is actually
+	 * assigned here (and unassigned below), but that's yet
+	 * another change to all drivers to add assign/unassign
+	 * emulation callbacks. Maybe later.
+	 */
+	if (sdata->vif.type =3D=3D NL80211_IFTYPE_MONITOR &&
+	    local->emulate_chanctx &&
+	    !ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
+		return 0;
+
 	if (!check_sdata_in_driver(sdata))
 		return -EIO;
=20
@@ -338,6 +350,11 @@ void drv_unassign_vif_chanctx(struct ieee80211_local *=
local,
 	might_sleep();
 	lockdep_assert_wiphy(local->hw.wiphy);
=20
+	if (sdata->vif.type =3D=3D NL80211_IFTYPE_MONITOR &&
+	    local->emulate_chanctx &&
+	    !ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
+		return;
+
 	if (!check_sdata_in_driver(sdata))
 		return;
=20
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index dc42902e2693..4a49e834a9f5 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1121,9 +1121,6 @@ int ieee80211_add_virtual_monitor(struct ieee80211_lo=
cal *local)
 	struct ieee80211_sub_if_data *sdata;
 	int ret;
=20
-	if (!ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
-		return 0;
-
 	ASSERT_RTNL();
 	lockdep_assert_wiphy(local->hw.wiphy);
=20
@@ -1145,11 +1142,13 @@ int ieee80211_add_virtual_monitor(struct ieee80211_=
local *local)
=20
 	ieee80211_set_default_queues(sdata);
=20
-	ret =3D drv_add_interface(local, sdata);
-	if (WARN_ON(ret)) {
-		/* ok .. stupid driver, it asked for this! */
-		kfree(sdata);
-		return ret;
+	if (ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
+		ret =3D drv_add_interface(local, sdata);
+		if (WARN_ON(ret)) {
+			/* ok .. stupid driver, it asked for this! */
+			kfree(sdata);
+			return ret;
+		}
 	}
=20
 	set_bit(SDATA_STATE_RUNNING, &sdata->state);
@@ -1187,9 +1186,6 @@ void ieee80211_del_virtual_monitor(struct ieee80211_l=
ocal *local)
 {
 	struct ieee80211_sub_if_data *sdata;
=20
-	if (!ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
-		return;
-
 	ASSERT_RTNL();
 	lockdep_assert_wiphy(local->hw.wiphy);
=20
@@ -1209,7 +1205,8 @@ void ieee80211_del_virtual_monitor(struct ieee80211_l=
ocal *local)
=20
 	ieee80211_link_release_channel(&sdata->deflink);
=20
-	drv_remove_interface(local, sdata);
+	if (ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
+		drv_remove_interface(local, sdata);
=20
 	kfree(sdata);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 283bfc99417e..963ed75deb76 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1843,7 +1843,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
=20
 	/* add interfaces */
 	sdata =3D wiphy_dereference(local->hw.wiphy, local->monitor_sdata);
-	if (sdata) {
+	if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
 		/* in HW restart it exists already */
 		WARN_ON(local->resuming);
 		res =3D drv_add_interface(local, sdata);