[PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[Weilun Du]

This fixed the crash when setting channels to 2 or more when
communicating over virtio.

Signed-off-by: Weilun Du <wdu@google.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 48 +++++++++++++++++++++------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 51ce767eaf88..0c44ec0ab03f 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -626,6 +626,7 @@ struct mac80211_hwsim_data {
 	u32 ciphers[ARRAY_SIZE(hwsim_ciphers)];
 
 	struct mac_address addresses[2];
+	struct ieee80211_chanctx_conf *chanctx;
 	int channels, idx;
 	bool use_chanctx;
 	bool destroy_on_close;
@@ -1257,7 +1258,8 @@ static inline u16 trans_tx_rate_flags_ieee2hwsim(struct ieee80211_tx_rate *rate)
 
 static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 				       struct sk_buff *my_skb,
-				       int dst_portid)
+				       int dst_portid,
+				       struct ieee80211_channel *channel)
 {
 	struct sk_buff *skb;
 	struct mac80211_hwsim_data *data = hw->priv;
@@ -1312,7 +1314,7 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 	if (nla_put_u32(skb, HWSIM_ATTR_FLAGS, hwsim_flags))
 		goto nla_put_failure;
 
-	if (nla_put_u32(skb, HWSIM_ATTR_FREQ, data->channel->center_freq))
+	if (nla_put_u32(skb, HWSIM_ATTR_FREQ, channel->center_freq))
 		goto nla_put_failure;
 
 	/* We get the tx control (rate and retries) info*/
@@ -1659,7 +1661,7 @@ static void mac80211_hwsim_tx(struct ieee80211_hw *hw,
 	_portid = READ_ONCE(data->wmediumd);
 
 	if (_portid || hwsim_virtio_enabled)
-		return mac80211_hwsim_tx_frame_nl(hw, skb, _portid);
+		return mac80211_hwsim_tx_frame_nl(hw, skb, _portid, channel);
 
 	/* NO wmediumd detected, perfect medium simulation */
 	data->tx_pkts++;
@@ -1770,7 +1772,7 @@ static void mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 	mac80211_hwsim_monitor_rx(hw, skb, chan);
 
 	if (_pid || hwsim_virtio_enabled)
-		return mac80211_hwsim_tx_frame_nl(hw, skb, _pid);
+		return mac80211_hwsim_tx_frame_nl(hw, skb, _pid, chan);
 
 	mac80211_hwsim_tx_frame_no_nl(hw, skb, chan);
 	dev_kfree_skb(skb);
@@ -2509,6 +2511,11 @@ static int mac80211_hwsim_croc(struct ieee80211_hw *hw,
 static int mac80211_hwsim_add_chanctx(struct ieee80211_hw *hw,
 				      struct ieee80211_chanctx_conf *ctx)
 {
+	struct mac80211_hwsim_data *hwsim = hw->priv;
+
+	mutex_lock(&hwsim->mutex);
+	hwsim->chanctx = ctx;
+	mutex_unlock(&hwsim->mutex);
 	hwsim_set_chanctx_magic(ctx);
 	wiphy_dbg(hw->wiphy,
 		  "add channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
@@ -2520,6 +2527,11 @@ static int mac80211_hwsim_add_chanctx(struct ieee80211_hw *hw,
 static void mac80211_hwsim_remove_chanctx(struct ieee80211_hw *hw,
 					  struct ieee80211_chanctx_conf *ctx)
 {
+	struct mac80211_hwsim_data *hwsim = hw->priv;
+
+	mutex_lock(&hwsim->mutex);
+	hwsim->chanctx = NULL;
+	mutex_unlock(&hwsim->mutex);
 	wiphy_dbg(hw->wiphy,
 		  "remove channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
 		  ctx->def.chan->center_freq, ctx->def.width,
@@ -2532,6 +2544,11 @@ static void mac80211_hwsim_change_chanctx(struct ieee80211_hw *hw,
 					  struct ieee80211_chanctx_conf *ctx,
 					  u32 changed)
 {
+	struct mac80211_hwsim_data *hwsim = hw->priv;
+
+	mutex_lock(&hwsim->mutex);
+	hwsim->chanctx = ctx;
+	mutex_unlock(&hwsim->mutex);
 	hwsim_check_chanctx_magic(ctx);
 	wiphy_dbg(hw->wiphy,
 		  "change channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
@@ -3124,6 +3141,7 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 		hw->wiphy->max_remain_on_channel_duration = 1000;
 		data->if_combination.radar_detect_widths = 0;
 		data->if_combination.num_different_channels = data->channels;
+		data->chanctx = NULL;
 	} else {
 		data->if_combination.num_different_channels = 1;
 		data->if_combination.radar_detect_widths =
@@ -3633,6 +3651,7 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	int frame_data_len;
 	void *frame_data;
 	struct sk_buff *skb = NULL;
+	struct ieee80211_channel *channel = NULL;
 
 	if (!info->attrs[HWSIM_ATTR_ADDR_RECEIVER] ||
 	    !info->attrs[HWSIM_ATTR_FRAME] ||
@@ -3659,6 +3678,17 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 	if (!data2)
 		goto out;
 
+	if (data2->use_chanctx) {
+		if (data2->tmp_chan)
+			channel = data2->tmp_chan;
+		else if (data2->chanctx)
+			channel = data2->chanctx->def.chan;
+	} else {
+		channel = data2->channel;
+	}
+	if (!channel)
+		goto out;
+
 	if (!hwsim_virtio_enabled) {
 		if (hwsim_net_get_netgroup(genl_info_net(info)) !=
 		    data2->netgroup)
@@ -3670,7 +3700,7 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 
 	/* check if radio is configured properly */
 
-	if (data2->idle || !data2->started)
+	if ((data2->idle && !data2->tmp_chan) || !data2->started)
 		goto out;
 
 	/* A frame is received from user space */
@@ -3683,18 +3713,16 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
 		mutex_lock(&data2->mutex);
 		rx_status.freq = nla_get_u32(info->attrs[HWSIM_ATTR_FREQ]);
 
-		if (rx_status.freq != data2->channel->center_freq &&
-		    (!data2->tmp_chan ||
-		     rx_status.freq != data2->tmp_chan->center_freq)) {
+		if (rx_status.freq != channel->center_freq) {
 			mutex_unlock(&data2->mutex);
 			goto out;
 		}
 		mutex_unlock(&data2->mutex);
 	} else {
-		rx_status.freq = data2->channel->center_freq;
+		rx_status.freq = channel->center_freq;
 	}
 
-	rx_status.band = data2->channel->band;
+	rx_status.band = channel->band;
 	rx_status.rate_idx = nla_get_u32(info->attrs[HWSIM_ATTR_RX_RATE]);
 	rx_status.signal = nla_get_u32(info->attrs[HWSIM_ATTR_SIGNAL]);
 
-- 
2.31.1.607.g51e8a6a459-goog

Re: [PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[Johannes Berg]

On Thu, 2021-05-06 at 11:05 -0700, Weilun Du wrote:
> This fixed the crash when setting channels to 2 or more when
> communicating over virtio.

Interesting, I thought I was probably the only user of virtio? :)

johannes

Re: [PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[Weilun Du]

On Thu, May 6, 2021 at 11:19 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Thu, 2021-05-06 at 11:05 -0700, Weilun Du wrote:
> > This fixed the crash when setting channels to 2 or more when
> > communicating over virtio.
>
> Interesting, I thought I was probably the only user of virtio? :)
>
> johannes
>
Hi Johannes,
Actually, Android Emulator uses mac80211_hwsim for wifi simulation
over virtio and it's working. This patch fixed the crash when we set
channels=2 to speed up scanning. I am trying to see if it makes sense
to upstream this patch since it's not Android-specific. Thanks!

Re: [PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[Johannes Berg]

On Thu, 2021-05-06 at 14:11 -0700, Weilun Du wrote:
> On Thu, May 6, 2021 at 11:19 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > On Thu, 2021-05-06 at 11:05 -0700, Weilun Du wrote:
> > > This fixed the crash when setting channels to 2 or more when
> > > communicating over virtio.
> > 
> > Interesting, I thought I was probably the only user of virtio? :)
> > 
> > johannes
> > 
> Hi Johannes,
> Actually, Android Emulator uses mac80211_hwsim for wifi simulation
> over virtio and it's working. This patch fixed the crash when we set
> channels=2 to speed up scanning. I am trying to see if it makes sense
> to upstream this patch since it's not Android-specific. Thanks!
> 
Oh sure, I'll take a look and will probably apply it for the next cycle,
haven't done an in-depth review. I was just surprised somebody actually
*used* the virtio thing :-)

Does that mean virt_wifi isn't used anymore by the Android emulator?
Maybe then we should remove that again, it had some obscure bugs that
syzkaller found? Not that I mind it being around much, but ...

johannes

Re: [PATCH v1] mac80211_hwsim: add concurrent channels scanning support over virtio

[Weilun Du]

On Thu, May 6, 2021 at 2:13 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Thu, 2021-05-06 at 14:11 -0700, Weilun Du wrote:
> > On Thu, May 6, 2021 at 11:19 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > >
> > > On Thu, 2021-05-06 at 11:05 -0700, Weilun Du wrote:
> > > > This fixed the crash when setting channels to 2 or more when
> > > > communicating over virtio.
> > >
> > > Interesting, I thought I was probably the only user of virtio? :)
> > >
> > > johannes
> > >
> > Hi Johannes,
> > Actually, Android Emulator uses mac80211_hwsim for wifi simulation
> > over virtio and it's working. This patch fixed the crash when we set
> > channels=2 to speed up scanning. I am trying to see if it makes sense
> > to upstream this patch since it's not Android-specific. Thanks!
> >
> Oh sure, I'll take a look and will probably apply it for the next cycle,
> haven't done an in-depth review. I was just surprised somebody actually
> *used* the virtio thing :-)
>
> Does that mean virt_wifi isn't used anymore by the Android emulator?
> Maybe then we should remove that again, it had some obscure bugs that
> syzkaller found? Not that I mind it being around much, but ...
>
> johannes
>

Thanks for looking into it. Actually, virt_wifi is still in use. There
are two different emulators that target different use cases. Android
emulator (goldfish) is mainly for third-party app developers. But yes
eventually, we are going to converge the Wi-Fi solution.