Message ID | 20240412025149.1211-1-quic_kangyang@quicinc.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Series | [v2] wifi: ath12k: add support to handle beacon miss for WCN7850 | expand |
On Fri Apr 12, 2024 at 4:51 AM CEST, kangyang wrote: [...] > @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif) > lockdep_assert_held(&ar->conf_mutex); > > arvif->ar = ar; > + arvif->vif = vif; > + > + INIT_LIST_HEAD(&arvif->list); > + INIT_DELAYED_WORK(&arvif->connection_loss_work, > + ath12k_mac_vif_sta_connection_loss_work); > + Is there a need to move the following part ? Isn't just adding the delay work enough ? > + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > + arvif->bitrate_mask.control[i].legacy = 0xffffffff; > + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > + sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > + sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > + } > + > vdev_id = __ffs64(ab->free_vdev_map); > arvif->vdev_id = vdev_id; > arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE; > @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw, > > arvif->vif = vif; > > - INIT_LIST_HEAD(&arvif->list); > - > - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > - arvif->bitrate_mask.control[i].legacy = 0xffffffff; > - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > - sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > - sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > - } > - > /* Allocate Default Queue now and reassign during actual vdev create */ > vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE; > for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) [...] Thanks
On 4/12/2024 3:33 PM, Nicolas Escande wrote: > On Fri Apr 12, 2024 at 4:51 AM CEST, kangyang wrote: > [...] >> @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif) >> lockdep_assert_held(&ar->conf_mutex); >> >> arvif->ar = ar; >> + arvif->vif = vif; >> + >> + INIT_LIST_HEAD(&arvif->list); >> + INIT_DELAYED_WORK(&arvif->connection_loss_work, >> + ath12k_mac_vif_sta_connection_loss_work); >> + > Is there a need to move the following part ? > Isn't just adding the delay work enough ? Just checked, you are right, but should add delay work in add_interface(). Will change in v3. >> + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { >> + arvif->bitrate_mask.control[i].legacy = 0xffffffff; >> + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, >> + sizeof(arvif->bitrate_mask.control[i].ht_mcs)); >> + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, >> + sizeof(arvif->bitrate_mask.control[i].vht_mcs)); >> + } >> + >> vdev_id = __ffs64(ab->free_vdev_map); >> arvif->vdev_id = vdev_id; >> arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE; >> @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw, >> >> arvif->vif = vif; >> >> - INIT_LIST_HEAD(&arvif->list); >> - >> - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { >> - arvif->bitrate_mask.control[i].legacy = 0xffffffff; >> - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, >> - sizeof(arvif->bitrate_mask.control[i].ht_mcs)); >> - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, >> - sizeof(arvif->bitrate_mask.control[i].vht_mcs)); >> - } >> - >> /* Allocate Default Queue now and reassign during actual vdev create */ >> vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE; >> for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) > [...] > > Thanks
On Fri Apr 12, 2024 at 10:47 AM CEST, Kang Yang wrote: > > > On 4/12/2024 3:33 PM, Nicolas Escande wrote: > > On Fri Apr 12, 2024 at 4:51 AM CEST, kangyang wrote: > > [...] > >> @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif) > >> lockdep_assert_held(&ar->conf_mutex); > >> > >> arvif->ar = ar; > >> + arvif->vif = vif; > >> + > >> + INIT_LIST_HEAD(&arvif->list); > >> + INIT_DELAYED_WORK(&arvif->connection_loss_work, > >> + ath12k_mac_vif_sta_connection_loss_work); > >> + > > Is there a need to move the following part ? > > Isn't just adding the delay work enough ? > > > Just checked, you are right, but should add delay work in add_interface(). > > Will change in v3. > > > >> + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > >> + arvif->bitrate_mask.control[i].legacy = 0xffffffff; > >> + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > >> + sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > >> + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > >> + sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > >> + } > >> + > >> vdev_id = __ffs64(ab->free_vdev_map); > >> arvif->vdev_id = vdev_id; > >> arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE; > >> @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw, > >> > >> arvif->vif = vif; > >> > >> - INIT_LIST_HEAD(&arvif->list); > >> - > >> - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > >> - arvif->bitrate_mask.control[i].legacy = 0xffffffff; > >> - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > >> - sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > >> - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > >> - sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > >> - } > >> - > >> /* Allocate Default Queue now and reassign during actual vdev create */ > >> vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE; > >> for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) > > [...] > > > > Thanks Yeah, I wasn't clear enough, I meant adding the INIT_DELAY_WORK without moving the rest of the code around. Thanks
On 4/12/24 5:23 PM, "Nicolas Escande" <nico.escande@gmail.com> wrote: > On Fri Apr 12, 2024 at 10:47 AM CEST, Kang Yang wrote: > > > > > > On 4/12/2024 3:33 PM, Nicolas Escande wrote: > >> On Fri Apr 12, 2024 at 4:51 AM CEST, kangyang wrote: > >> [...] > >>> @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif) > >>> lockdep_assert_held(&ar->conf_mutex); > >>> > >>> arvif->ar = ar; > >>> + arvif->vif = vif; > >>> + > >>> + INIT_LIST_HEAD(&arvif->list); > >>> + INIT_DELAYED_WORK(&arvif->connection_loss_work, > >>> + ath12k_mac_vif_sta_connection_loss_work); > >>> + > >> Is there a need to move the following part ? > >> Isn't just adding the delay work enough ? > > > > > > Just checked, you are right, but should add delay work in add_interface(). > > > > Will change in v3. > > > > > >>> + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > >>> + arvif->bitrate_mask.control[i].legacy = 0xffffffff; > >>> + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > >>> + sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > >>> + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > >>> + sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > >>> + } > >>> + > >>> vdev_id = __ffs64(ab->free_vdev_map); > >>> arvif->vdev_id = vdev_id; > >>> arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE; > >>> @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw, > >>> > >>> arvif->vif = vif; > >>> > >>> - INIT_LIST_HEAD(&arvif->list); > >>> - > >>> - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { > >>> - arvif->bitrate_mask.control[i].legacy = 0xffffffff; > >>> - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, > >>> - sizeof(arvif->bitrate_mask.control[i].ht_mcs)); > >>> - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, > >>> - sizeof(arvif->bitrate_mask.control[i].vht_mcs)); > >>> - } > >>> - > >>> /* Allocate Default Queue now and reassign during actual vdev create */ > >>> vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE; > >>> for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) > >> [...] > >> > >> Thanks > > Yeah, I wasn't clear enough, I meant adding the INIT_DELAY_WORK without moving > the rest of the code around. > I know what you mean --- Just add INIT_DELAY_WORK in ath12k_mac_vdev_create(). But i just checked with the person who just refactor the related code in patchset 'wifi: ath12k: Add single wiphy support'. He also think add in ath12k_mac_op_add_interface() is better. > Thanks >
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h index 397d8c973265..e125efe20dde 100644 --- a/drivers/net/wireless/ath/ath12k/core.h +++ b/drivers/net/wireless/ath/ath12k/core.h @@ -46,6 +46,7 @@ #define ATH12K_SMBIOS_BDF_EXT_MAGIC "BDF_" #define ATH12K_INVALID_HW_MAC_ID 0xFF +#define ATH12K_CONNECTION_LOSS_HZ (3 * HZ) #define ATH12K_RX_RATE_TABLE_NUM 320 #define ATH12K_RX_RATE_TABLE_11AX_NUM 576 @@ -275,6 +276,7 @@ struct ath12k_vif { u32 aid; u8 bssid[ETH_ALEN]; struct cfg80211_bitrate_mask bitrate_mask; + struct delayed_work connection_loss_work; int num_legacy_stations; int rtscts_prot_mode; int txpower; diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index f15dcd75157d..decd7f2840d5 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -1398,6 +1398,75 @@ static void ath12k_control_beaconing(struct ath12k_vif *arvif, ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev %d up\n", arvif->vdev_id); } +static void ath12k_mac_handle_beacon_iter(void *data, u8 *mac, + struct ieee80211_vif *vif) +{ + struct sk_buff *skb = data; + struct ieee80211_mgmt *mgmt = (void *)skb->data; + struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif); + + if (vif->type != NL80211_IFTYPE_STATION) + return; + + if (!ether_addr_equal(mgmt->bssid, vif->bss_conf.bssid)) + return; + + cancel_delayed_work(&arvif->connection_loss_work); +} + +void ath12k_mac_handle_beacon(struct ath12k *ar, struct sk_buff *skb) +{ + ieee80211_iterate_active_interfaces_atomic(ath12k_ar_to_hw(ar), + IEEE80211_IFACE_ITER_NORMAL, + ath12k_mac_handle_beacon_iter, + skb); +} + +static void ath12k_mac_handle_beacon_miss_iter(void *data, u8 *mac, + struct ieee80211_vif *vif) +{ + u32 *vdev_id = data; + struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif); + struct ath12k *ar = arvif->ar; + struct ieee80211_hw *hw = ath12k_ar_to_hw(ar); + + if (arvif->vdev_id != *vdev_id) + return; + + if (!arvif->is_up) + return; + + ieee80211_beacon_loss(vif); + + /* Firmware doesn't report beacon loss events repeatedly. If AP probe + * (done by mac80211) succeeds but beacons do not resume then it + * doesn't make sense to continue operation. Queue connection loss work + * which can be cancelled when beacon is received. + */ + ieee80211_queue_delayed_work(hw, &arvif->connection_loss_work, + ATH12K_CONNECTION_LOSS_HZ); +} + +void ath12k_mac_handle_beacon_miss(struct ath12k *ar, u32 vdev_id) +{ + ieee80211_iterate_active_interfaces_atomic(ath12k_ar_to_hw(ar), + IEEE80211_IFACE_ITER_NORMAL, + ath12k_mac_handle_beacon_miss_iter, + &vdev_id); +} + +static void ath12k_mac_vif_sta_connection_loss_work(struct work_struct *work) +{ + struct ath12k_vif *arvif = container_of(work, struct ath12k_vif, + connection_loss_work.work); + struct ieee80211_vif *vif = arvif->vif; + + if (!arvif->is_up) + return; + + ieee80211_connection_loss(vif); +} + static void ath12k_peer_assoc_h_basic(struct ath12k *ar, struct ieee80211_vif *vif, struct ieee80211_sta *sta, @@ -2570,7 +2639,7 @@ static void ath12k_bss_disassoc(struct ath12k *ar, arvif->is_up = false; - /* TODO: cancel connection_loss_work */ + cancel_delayed_work(&arvif->connection_loss_work); } static u32 ath12k_mac_get_rate_hw_value(int bitrate) @@ -5986,6 +6055,20 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif) lockdep_assert_held(&ar->conf_mutex); arvif->ar = ar; + arvif->vif = vif; + + INIT_LIST_HEAD(&arvif->list); + INIT_DELAYED_WORK(&arvif->connection_loss_work, + ath12k_mac_vif_sta_connection_loss_work); + + for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { + arvif->bitrate_mask.control[i].legacy = 0xffffffff; + memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, + sizeof(arvif->bitrate_mask.control[i].ht_mcs)); + memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, + sizeof(arvif->bitrate_mask.control[i].vht_mcs)); + } + vdev_id = __ffs64(ab->free_vdev_map); arvif->vdev_id = vdev_id; arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE; @@ -6316,16 +6399,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw, arvif->vif = vif; - INIT_LIST_HEAD(&arvif->list); - - for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) { - arvif->bitrate_mask.control[i].legacy = 0xffffffff; - memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff, - sizeof(arvif->bitrate_mask.control[i].ht_mcs)); - memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff, - sizeof(arvif->bitrate_mask.control[i].vht_mcs)); - } - /* Allocate Default Queue now and reassign during actual vdev create */ vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE; for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++) @@ -6451,6 +6524,8 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw, mutex_lock(&ar->conf_mutex); + cancel_delayed_work_sync(&arvif->connection_loss_work); + ath12k_dbg(ab, ATH12K_DBG_MAC, "mac remove interface (vdev %d)\n", arvif->vdev_id); diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h index 3f5e1be0dff9..bfc655a4dfce 100644 --- a/drivers/net/wireless/ath/ath12k/mac.h +++ b/drivers/net/wireless/ath/ath12k/mac.h @@ -78,4 +78,6 @@ enum ath12k_supported_bw ath12k_mac_mac80211_bw_to_ath12k_bw(enum rate_info_bw b enum hal_encrypt_type ath12k_dp_tx_get_encrypt_type(u32 cipher); int ath12k_mac_rfkill_enable_radio(struct ath12k *ar, bool enable); int ath12k_mac_rfkill_config(struct ath12k *ar); +void ath12k_mac_handle_beacon(struct ath12k *ar, struct sk_buff *skb); +void ath12k_mac_handle_beacon_miss(struct ath12k *ar, u32 vdev_id); #endif diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c index a5575ce9eed4..4fe39e920902 100644 --- a/drivers/net/wireless/ath/ath12k/wmi.c +++ b/drivers/net/wireless/ath/ath12k/wmi.c @@ -5927,10 +5927,8 @@ static void ath12k_mgmt_rx_event(struct ath12k_base *ab, struct sk_buff *skb) } } - /* TODO: Pending handle beacon implementation - *if (ieee80211_is_beacon(hdr->frame_control)) - * ath12k_mac_handle_beacon(ar, skb); - */ + if (ieee80211_is_beacon(hdr->frame_control)) + ath12k_mac_handle_beacon(ar, skb); ath12k_dbg(ab, ATH12K_DBG_MGMT, "event mgmt rx skb %pK len %d ftype %02x stype %02x\n", @@ -6137,42 +6135,44 @@ static void ath12k_roam_event(struct ath12k_base *ab, struct sk_buff *skb) { struct wmi_roam_event roam_ev = {}; struct ath12k *ar; + u32 vdev_id; + u8 roam_reason; if (ath12k_pull_roam_ev(ab, skb, &roam_ev) != 0) { ath12k_warn(ab, "failed to extract roam event"); return; } + vdev_id = le32_to_cpu(roam_ev.vdev_id); + roam_reason = u32_get_bits(le32_to_cpu(roam_ev.reason), + WMI_ROAM_REASON_MASK); + ath12k_dbg(ab, ATH12K_DBG_WMI, - "wmi roam event vdev %u reason 0x%08x rssi %d\n", - roam_ev.vdev_id, roam_ev.reason, roam_ev.rssi); + "wmi roam event vdev %u reason %d rssi %d\n", + vdev_id, roam_reason, roam_ev.rssi); rcu_read_lock(); - ar = ath12k_mac_get_ar_by_vdev_id(ab, le32_to_cpu(roam_ev.vdev_id)); + ar = ath12k_mac_get_ar_by_vdev_id(ab, vdev_id); if (!ar) { - ath12k_warn(ab, "invalid vdev id in roam ev %d", - roam_ev.vdev_id); + ath12k_warn(ab, "invalid vdev id in roam ev %d", vdev_id); rcu_read_unlock(); return; } - if (le32_to_cpu(roam_ev.reason) >= WMI_ROAM_REASON_MAX) + if (roam_reason >= WMI_ROAM_REASON_MAX) ath12k_warn(ab, "ignoring unknown roam event reason %d on vdev %i\n", - roam_ev.reason, roam_ev.vdev_id); + roam_reason, vdev_id); - switch (le32_to_cpu(roam_ev.reason)) { + switch (roam_reason) { case WMI_ROAM_REASON_BEACON_MISS: - /* TODO: Pending beacon miss and connection_loss_work - * implementation - * ath12k_mac_handle_beacon_miss(ar, vdev_id); - */ + ath12k_mac_handle_beacon_miss(ar, vdev_id); break; case WMI_ROAM_REASON_BETTER_AP: case WMI_ROAM_REASON_LOW_RSSI: case WMI_ROAM_REASON_SUITABLE_AP_FOUND: case WMI_ROAM_REASON_HO_FAILED: ath12k_warn(ab, "ignoring not implemented roam event reason %d on vdev %i\n", - roam_ev.reason, roam_ev.vdev_id); + roam_reason, vdev_id); break; } diff --git a/drivers/net/wireless/ath/ath12k/wmi.h b/drivers/net/wireless/ath/ath12k/wmi.h index 78afc94a815d..ebf73ddcadc6 100644 --- a/drivers/net/wireless/ath/ath12k/wmi.h +++ b/drivers/net/wireless/ath/ath12k/wmi.h @@ -4182,6 +4182,9 @@ struct wmi_peer_sta_kickout_event { struct ath12k_wmi_mac_addr_params peer_macaddr; } __packed; +#define WMI_ROAM_REASON_MASK GENMASK(3, 0) +#define WMI_ROAM_SUBNET_STATUS_MASK GENMASK(5, 4) + enum wmi_roam_reason { WMI_ROAM_REASON_BETTER_AP = 1, WMI_ROAM_REASON_BEACON_MISS = 2,