| Message ID | 20250107021017.3857555-8-quic_periyasa@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath12k: Add monitor interface support on QCN9274 | expand |
On Tue Jan 7, 2025 at 3:10 AM CET, Karthikeyan Periyasamy wrote: > From: P Praneesh <quic_ppranees@quicinc.com> > > Currently, the monitor interface is not supported. To support the monitor > interface, configure the monitor vdev state identifier, configure the HTT > filter setup, subscribe the mac80211 WANT_MONITOR_VIF feature and prevent > monitor interface to transmit packet. Therefore, add these procedures to > add monitor interface support and enable the monitor interface support on > the QCN9274 platform through the hardware parameter. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 > Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 > > Signed-off-by: P Praneesh <quic_ppranees@quicinc.com> > Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> > --- [...] > diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c > index b8f0df6f7a05..81d1ff918f48 100644 > --- a/drivers/net/wireless/ath/ath12k/dp_tx.c > +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: BSD-3-Clause-Clear > /* > * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. > - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. > */ > > #include "core.h" > @@ -1277,6 +1277,8 @@ int ath12k_dp_tx_htt_rx_monitor_mode_ring_config(struct ath12k *ar, bool reset) > HTT_RX_MON_MO_CTRL_FILTER_FLASG3 | > HTT_RX_MON_FP_DATA_FILTER_FLASG3 | > HTT_RX_MON_MO_DATA_FILTER_FLASG3; > + } else { > + tlv_filter.rxmon_disable = true; Shouldn't this be tlv_filter = ath12k_mac_mon_status_filter_default; To match de default value ? > } > > if (ab->hw_params->rxdma1_enable) { > diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c > index a106ebed7870..021a4b565e8b 100644 > --- a/drivers/net/wireless/ath/ath12k/hw.c > +++ b/drivers/net/wireless/ath/ath12k/hw.c > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: BSD-3-Clause-Clear > /* > * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. > - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. > + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. > */ > > #include <linux/types.h> > @@ -1049,7 +1049,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { > BIT(NL80211_IFTYPE_AP) | > BIT(NL80211_IFTYPE_MESH_POINT) | > BIT(NL80211_IFTYPE_AP_VLAN), > - .supports_monitor = false, > + .supports_monitor = true, > > .idle_ps = false, > .download_calib = true, > diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > index abf7c7ed8967..27d44b508884 100644 > --- a/drivers/net/wireless/ath/ath12k/mac.c > +++ b/drivers/net/wireless/ath/ath12k/mac.c > @@ -1264,6 +1264,12 @@ static int ath12k_mac_monitor_start(struct ath12k *ar) > return ret; > } > > + ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); This is already done a few lines after. Shouldn't the one after be removed > + if (ret) { > + ath12k_warn(ar->ab, "fail to set monitor filter: %d\n", ret); > + return ret; > + } > + > ar->monitor_started = true; > ar->num_started_vdevs++; > ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); [...] And this is probably out of the scope of this patch but the code in ath12k_dp_tx_htt_rx_monitor_mode_ring_config could be simplified by returning early if (!ab->hw_params->rxdma1_enable) like in ath12k_mac_config_mon_status_default. And that brings the question for me as shouldn't theese two functions should be refactored to share more code
On 1/17/2025 5:03 PM, Nicolas Escande wrote: > On Tue Jan 7, 2025 at 3:10 AM CET, Karthikeyan Periyasamy wrote: >> From: P Praneesh <quic_ppranees@quicinc.com> >> >> Currently, the monitor interface is not supported. To support the monitor >> interface, configure the monitor vdev state identifier, configure the HTT >> filter setup, subscribe the mac80211 WANT_MONITOR_VIF feature and prevent >> monitor interface to transmit packet. Therefore, add these procedures to >> add monitor interface support and enable the monitor interface support on >> the QCN9274 platform through the hardware parameter. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 >> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 >> >> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com> >> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> >> --- > [...] >> diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c >> index b8f0df6f7a05..81d1ff918f48 100644 >> --- a/drivers/net/wireless/ath/ath12k/dp_tx.c >> +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c >> @@ -1,7 +1,7 @@ >> // SPDX-License-Identifier: BSD-3-Clause-Clear >> /* >> * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. >> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. >> */ >> >> #include "core.h" >> @@ -1277,6 +1277,8 @@ int ath12k_dp_tx_htt_rx_monitor_mode_ring_config(struct ath12k *ar, bool reset) >> HTT_RX_MON_MO_CTRL_FILTER_FLASG3 | >> HTT_RX_MON_FP_DATA_FILTER_FLASG3 | >> HTT_RX_MON_MO_DATA_FILTER_FLASG3; >> + } else { >> + tlv_filter.rxmon_disable = true; > Shouldn't this be > tlv_filter = ath12k_mac_mon_status_filter_default; > To match de default value ? In standalone monitor case, this will enable the default filter unnecessary, no ? >> } >> >> if (ab->hw_params->rxdma1_enable) { >> diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c >> index a106ebed7870..021a4b565e8b 100644 >> --- a/drivers/net/wireless/ath/ath12k/hw.c >> +++ b/drivers/net/wireless/ath/ath12k/hw.c >> @@ -1,7 +1,7 @@ >> // SPDX-License-Identifier: BSD-3-Clause-Clear >> /* >> * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. >> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. >> */ >> >> #include <linux/types.h> >> @@ -1049,7 +1049,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { >> BIT(NL80211_IFTYPE_AP) | >> BIT(NL80211_IFT YPE_MESH_POINT) | >> BIT(NL80211_IFTYPE_AP_VLAN), >> - .supports_monitor = false, >> + .supports_monitor = true, >> >> .idle_ps = false, >> .download_calib = true, >> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c >> index abf7c7ed8967..27d44b508884 100644 >> --- a/drivers/net/wireless/ath/ath12k/mac.c >> +++ b/drivers/net/wireless/ath/ath12k/mac.c >> @@ -1264,6 +1264,12 @@ static int ath12k_mac_monitor_start(struct ath12k *ar) >> return ret; >> } >> >> + ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); > This is already done a few lines after. Shouldn't the one after be removed yes, will remove in the next version of the patch.
On Fri Jan 17, 2025 at 2:06 PM CET, Karthikeyan Periyasamy wrote: > > > On 1/17/2025 5:03 PM, Nicolas Escande wrote: > > On Tue Jan 7, 2025 at 3:10 AM CET, Karthikeyan Periyasamy wrote: > >> From: P Praneesh <quic_ppranees@quicinc.com> > >> > >> Currently, the monitor interface is not supported. To support the monitor > >> interface, configure the monitor vdev state identifier, configure the HTT > >> filter setup, subscribe the mac80211 WANT_MONITOR_VIF feature and prevent > >> monitor interface to transmit packet. Therefore, add these procedures to > >> add monitor interface support and enable the monitor interface support on > >> the QCN9274 platform through the hardware parameter. > >> > >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1 > >> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3 > >> > >> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com> > >> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> > >> --- > > [...] > >> diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c > >> index b8f0df6f7a05..81d1ff918f48 100644 > >> --- a/drivers/net/wireless/ath/ath12k/dp_tx.c > >> +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c > >> @@ -1,7 +1,7 @@ > >> // SPDX-License-Identifier: BSD-3-Clause-Clear > >> /* > >> * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. > >> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. > >> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. > >> */ > >> > >> #include "core.h" > >> @@ -1277,6 +1277,8 @@ int ath12k_dp_tx_htt_rx_monitor_mode_ring_config(struct ath12k *ar, bool reset) > >> HTT_RX_MON_MO_CTRL_FILTER_FLASG3 | > >> HTT_RX_MON_FP_DATA_FILTER_FLASG3 | > >> HTT_RX_MON_MO_DATA_FILTER_FLASG3; > >> + } else { > >> + tlv_filter.rxmon_disable = true; > > Shouldn't this be > > tlv_filter = ath12k_mac_mon_status_filter_default; > > To match de default value ? > > In standalone monitor case, this will enable the default filter > unnecessary, no ? I'm not too familiar with the driver but clearly this is the part that breaks sta info update when running a monitor while an ap is running. And as this is a valid interface combination it should work right ? So this change works around the problem I reported earlier, I just confirmed it on my setup. But maybe this is not the right fix indeed but hopefully it points towards the culprit. > > > >> } > >> > >> if (ab->hw_params->rxdma1_enable) { > >> diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c > >> index a106ebed7870..021a4b565e8b 100644 > >> --- a/drivers/net/wireless/ath/ath12k/hw.c > >> +++ b/drivers/net/wireless/ath/ath12k/hw.c > >> @@ -1,7 +1,7 @@ > >> // SPDX-License-Identifier: BSD-3-Clause-Clear > >> /* > >> * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. > >> - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. > >> + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. > >> */ > >> > >> #include <linux/types.h> > >> @@ -1049,7 +1049,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { > >> BIT(NL80211_IFTYPE_AP) | > >> BIT(NL80211_IFT > > YPE_MESH_POINT) | > >> BIT(NL80211_IFTYPE_AP_VLAN), > >> - .supports_monitor = false, > >> + .supports_monitor = true, > >> > >> .idle_ps = false, > >> .download_calib = true, > >> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c > >> index abf7c7ed8967..27d44b508884 100644 > >> --- a/drivers/net/wireless/ath/ath12k/mac.c > >> +++ b/drivers/net/wireless/ath/ath12k/mac.c > >> @@ -1264,6 +1264,12 @@ static int ath12k_mac_monitor_start(struct ath12k *ar) > >> return ret; > >> } > >> > >> + ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); > > This is already done a few lines after. Shouldn't the one after be removed > > yes, will remove in the next version of the patch.
On Tue Jan 21, 2025 at 10:12 AM CET, Karthikeyan Periyasamy wrote: > On 1/17/2025 6:50 PM, Nicolas Escande wrote: [...] >> I'm not too familiar with the driver but clearly this is the part that breaks >> sta info update when running a monitor while an ap is running. >> >> And as this is a valid interface combination it should work right ? >> >> So this change works around the problem I reported earlier, I just confirmed it >> on my setup. But maybe this is not the right fix indeed but hopefully it >> points towards the culprit. >> > > Addressed in the next version of the patch. Yes thank you for that. Everything is working fine now.
diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c index 0c6b35aac96e..3a194cc52598 100644 --- a/drivers/net/wireless/ath/ath12k/core.c +++ b/drivers/net/wireless/ath/ath12k/core.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause-Clear /* * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. */ #include <linux/module.h> @@ -1261,6 +1261,11 @@ static void ath12k_core_pre_reconfigure_recovery(struct ath12k_base *ab) ath12k_mac_tx_mgmt_pending_free, ar); idr_destroy(&ar->txmgmt_idr); wake_up(&ar->txmgmt_empty_waitq); + + ar->monitor_vdev_id = -1; + ar->monitor_conf_enabled = false; + ar->monitor_vdev_created = false; + ar->monitor_started = false; } } diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c index b8f0df6f7a05..81d1ff918f48 100644 --- a/drivers/net/wireless/ath/ath12k/dp_tx.c +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause-Clear /* * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. */ #include "core.h" @@ -1277,6 +1277,8 @@ int ath12k_dp_tx_htt_rx_monitor_mode_ring_config(struct ath12k *ar, bool reset) HTT_RX_MON_MO_CTRL_FILTER_FLASG3 | HTT_RX_MON_FP_DATA_FILTER_FLASG3 | HTT_RX_MON_MO_DATA_FILTER_FLASG3; + } else { + tlv_filter.rxmon_disable = true; } if (ab->hw_params->rxdma1_enable) { diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c index a106ebed7870..021a4b565e8b 100644 --- a/drivers/net/wireless/ath/ath12k/hw.c +++ b/drivers/net/wireless/ath/ath12k/hw.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause-Clear /* * Copyright (c) 2018-2021 The Linux Foundation. All rights reserved. - * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved. */ #include <linux/types.h> @@ -1049,7 +1049,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { BIT(NL80211_IFTYPE_AP) | BIT(NL80211_IFTYPE_MESH_POINT) | BIT(NL80211_IFTYPE_AP_VLAN), - .supports_monitor = false, + .supports_monitor = true, .idle_ps = false, .download_calib = true, diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index abf7c7ed8967..27d44b508884 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -1264,6 +1264,12 @@ static int ath12k_mac_monitor_start(struct ath12k *ar) return ret; } + ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); + if (ret) { + ath12k_warn(ar->ab, "fail to set monitor filter: %d\n", ret); + return ret; + } + ar->monitor_started = true; ar->num_started_vdevs++; ret = ath12k_dp_tx_htt_monitor_mode_ring_config(ar, false); @@ -7170,6 +7176,11 @@ static void ath12k_mac_op_tx(struct ieee80211_hw *hw, u8 link_id; int ret; + if (ahvif->vdev_type == WMI_VDEV_TYPE_MONITOR) { + ieee80211_free_txskb(hw, skb); + return; + } + link_id = u32_get_bits(info->control.flags, IEEE80211_TX_CTRL_MLO_LINK); memset(skb_cb, 0, sizeof(*skb_cb)); skb_cb->vif = vif; @@ -7253,6 +7264,8 @@ static int ath12k_mac_config_mon_status_default(struct ath12k *ar, bool enable) else tlv_filter.rxmon_disable = true; + tlv_filter.offset_valid = false; + for (i = 0; i < ab->hw_params->num_rxdma_per_pdev; i++) { ring_id = ar->dp.rxdma_mon_dst_ring[i].ring_id; ret = ath12k_dp_tx_htt_rx_filter_setup(ab, ring_id, @@ -8021,6 +8034,9 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif) } err_vdev_del: + if (ahvif->vdev_type == WMI_VDEV_TYPE_MONITOR) + ar->monitor_vdev_id = -1; + ath12k_wmi_vdev_delete(ar, arvif->vdev_id); ar->num_created_vdevs--; arvif->is_created = false; @@ -10894,6 +10910,7 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah) ieee80211_hw_set(hw, QUEUE_CONTROL); ieee80211_hw_set(hw, SUPPORTS_TX_FRAG); ieee80211_hw_set(hw, REPORTS_LOW_ACK); + ieee80211_hw_set(hw, WANT_MONITOR_VIF); if ((ht_cap & WMI_HT_CAP_ENABLED) || is_6ghz) { ieee80211_hw_set(hw, AMPDU_AGGREGATION); @@ -11089,6 +11106,11 @@ static void ath12k_mac_setup(struct ath12k *ar) wiphy_work_init(&ar->wmi_mgmt_tx_work, ath12k_mgmt_over_wmi_tx_work); skb_queue_head_init(&ar->wmi_mgmt_tx_queue); + + ar->monitor_vdev_id = -1; + ar->monitor_conf_enabled = false; + ar->monitor_vdev_created = false; + ar->monitor_started = false; } static int __ath12k_mac_mlo_setup(struct ath12k *ar)