mbox series

[v3,0/8] wifi: ath12k: add support for WoW

Message ID 20240530072714.25671-1-quic_bqiang@quicinc.com
Headers show
Series wifi: ath12k: add support for WoW | expand

Message

Baochen Qiang May 30, 2024, 7:27 a.m. UTC
Add support for WoW (Wakeup on Wireless) funtionalities, this including
magic-pattern, net-detect, disconnect and patterns, HW data filter, ARP
and NS offload, GTK rekey offload.

Also enable keepalive before suspend to avoid unexpected kick out by AP.

v3:
 - wifi: ath12k: add ATH12K_DBG_WOW log level
	1. new addition.
 - wifi: ath12k: support ARP and NS offload
	1. move ipv6 infomation parse to ath12k_wow_arp_ns_offload() to
	   avoid memory leak issue.
	2. move ARP/NS related definition from core.h to wmi.h
	3. rename ath12k_arp_ns_offlaod to wmi_arp_ns_offload_arg
	4. rename some macros
 - wifi: ath12k: support GTK rekey offload
	1. add rcu_read_lock/unlock in ath12k_wmi_gtk_offload_status_event()


v2:
 - [2/7] wifi: ath12k: add basic WoW functionalities
        1. In ath12k_wow_convert_8023_to_80211(), change to use 'size_t'
           instead of 'int' to make GCC happy
 - [5/7] wifi: ath12k: support ARP and NS offload
        1. In ath12k_mac_arvif_get_arp_ns_offload(), change to use
           GFP_ATOMIC to fix kernel crash due to sleep in invalid context
 - rebased on ToT

Baochen Qiang (8):
  wifi: ath12k: add ATH12K_DBG_WOW log level
  wifi: ath12k: implement WoW enable and wakeup commands
  wifi: ath12k: add basic WoW functionalities
  wifi: ath12k: add WoW net-detect functionality
  wifi: ath12k: implement hardware data filter
  wifi: ath12k: support ARP and NS offload
  wifi: ath12k: support GTK rekey offload
  wifi: ath12k: handle keepalive during WoWLAN suspend and resume

 drivers/net/wireless/ath/ath12k/Makefile |    1 +
 drivers/net/wireless/ath/ath12k/core.c   |   48 +-
 drivers/net/wireless/ath/ath12k/core.h   |   20 +
 drivers/net/wireless/ath/ath12k/debug.h  |    1 +
 drivers/net/wireless/ath/ath12k/htc.c    |    6 +
 drivers/net/wireless/ath/ath12k/mac.c    |  110 +++
 drivers/net/wireless/ath/ath12k/mac.h    |    4 +
 drivers/net/wireless/ath/ath12k/wmi.c    |  720 +++++++++++++++
 drivers/net/wireless/ath/ath12k/wmi.h    |  578 ++++++++++++
 drivers/net/wireless/ath/ath12k/wow.c    | 1012 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath12k/wow.h    |   62 ++
 11 files changed, 2554 insertions(+), 8 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath12k/wow.c
 create mode 100644 drivers/net/wireless/ath/ath12k/wow.h


base-commit: 2580be9ee6f5d97d6763b5d4ae4f9c0383fdf130

Comments

Baochen Qiang May 31, 2024, 3:49 a.m. UTC | #1
On 5/31/2024 11:42 AM, Baochen Qiang wrote:
>> as noted above does it make more sense to get the netdev associated with the
>> arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
>> caching the pointer from the ipv6_addr_changed() callback?
> Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
> 
> will get it using the following in next version:
> 	struct ieee80211_vif *vif = container_of(arvif)
sorry, should be:
	struct ieee80211_vif *vif = arvif->vif

> 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
> 	struct net_dev *ndev = sub_if_data->dev
> 	struct inet6_dev *idev = in6_dev_get(ndev)
Baochen Qiang June 3, 2024, 2:47 a.m. UTC | #2
On 6/1/2024 1:26 AM, Jeff Johnson wrote:
> On 5/30/2024 10:11 PM, Baochen Qiang wrote:
>>
>>
>> On 5/31/2024 11:42 AM, Baochen Qiang wrote:
>>>>> +static void ath12k_wow_prepare_ns_offload(struct ath12k_vif *arvif,
>>>>> +					  struct wmi_arp_ns_offload_arg *offload)
>>>>> +{
>>>>> +	struct inet6_dev *idev = arvif->idev;
>>>> as noted above does it make more sense to get the netdev associated with the
>>>> arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
>>>> caching the pointer from the ipv6_addr_changed() callback?
>>> Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
>>>
>>> will get it using the following in next version:
>>> 	struct ieee80211_vif *vif = container_of(arvif)
>>> 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
>>> 	struct net_dev *ndev = sub_if_data->dev
>>> 	struct inet6_dev *idev = in6_dev_get(ndev)
>> Just found that ieee80211_sub_if_data is internal to mac80211, so not possible to get netdev in this way.
>>
>> any other ideas on how to get netdev?
> 
> Thinking about this some more, it seems like you'd want to send these down to
> firmware immediately so that they'd be available for NS offload. Does firmware
> support NS offload even when host is awake (I think the downstream Android
> driver supports that)?
Checked with firmware team and they confirmed it is supported. But do we really want to do this? Any benefit from offloading when awake?

And won't there be any issues? at least it would make me confused if a arp/NS request/response pair are seen in sniffer but neither of them seen in host ...
> So perhaps an alternative approach is to collect the information you need from
> the notification and then schedule a workqueue to actually send the
> information to firmware?
Johannes Berg June 3, 2024, 7:36 a.m. UTC | #3
On Fri, 2024-05-31 at 13:11 +0800, Baochen Qiang wrote:
> 
> On 5/31/2024 11:42 AM, Baochen Qiang wrote:
> > > > +static void ath12k_wow_prepare_ns_offload(struct ath12k_vif *arvif,
> > > > +					  struct wmi_arp_ns_offload_arg *offload)
> > > > +{
> > > > +	struct inet6_dev *idev = arvif->idev;
> > > as noted above does it make more sense to get the netdev associated with the
> > > arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
> > > caching the pointer from the ipv6_addr_changed() callback?
> > Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
> > 
> > will get it using the following in next version:
> > 	struct ieee80211_vif *vif = container_of(arvif)
> > 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
> > 	struct net_dev *ndev = sub_if_data->dev
> > 	struct inet6_dev *idev = in6_dev_get(ndev)
> Just found that ieee80211_sub_if_data is internal to mac80211, so not possible to get netdev in this way.
> 
> any other ideas on how to get netdev?

You can go via the wdev.

johannes
Baochen Qiang June 3, 2024, 9:15 a.m. UTC | #4
On 6/3/2024 3:36 PM, Johannes Berg wrote:
> On Fri, 2024-05-31 at 13:11 +0800, Baochen Qiang wrote:
>>
>> On 5/31/2024 11:42 AM, Baochen Qiang wrote:
>>>>> +static void ath12k_wow_prepare_ns_offload(struct ath12k_vif *arvif,
>>>>> +					  struct wmi_arp_ns_offload_arg *offload)
>>>>> +{
>>>>> +	struct inet6_dev *idev = arvif->idev;
>>>> as noted above does it make more sense to get the netdev associated with the
>>>> arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
>>>> caching the pointer from the ipv6_addr_changed() callback?
>>> Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
>>>
>>> will get it using the following in next version:
>>> 	struct ieee80211_vif *vif = container_of(arvif)
>>> 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
>>> 	struct net_dev *ndev = sub_if_data->dev
>>> 	struct inet6_dev *idev = in6_dev_get(ndev)
>> Just found that ieee80211_sub_if_data is internal to mac80211, so not possible to get netdev in this way.
>>
>> any other ideas on how to get netdev?
> 
> You can go via the wdev.
> 
Ah, great! I can get it with ieee80211_vif_to_wdev(arvif->vif)->netdev. Thank you for pointing that.

> johannes
Baochen Qiang June 3, 2024, 9:32 a.m. UTC | #5
On 6/1/2024 1:26 AM, Jeff Johnson wrote:
> On 5/30/2024 10:11 PM, Baochen Qiang wrote:
>>
>>
>> On 5/31/2024 11:42 AM, Baochen Qiang wrote:
>>>>> +static void ath12k_wow_prepare_ns_offload(struct ath12k_vif *arvif,
>>>>> +					  struct wmi_arp_ns_offload_arg *offload)
>>>>> +{
>>>>> +	struct inet6_dev *idev = arvif->idev;
>>>> as noted above does it make more sense to get the netdev associated with the
>>>> arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
>>>> caching the pointer from the ipv6_addr_changed() callback?
>>> Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
>>>
>>> will get it using the following in next version:
>>> 	struct ieee80211_vif *vif = container_of(arvif)
>>> 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
>>> 	struct net_dev *ndev = sub_if_data->dev
>>> 	struct inet6_dev *idev = in6_dev_get(ndev)
>> Just found that ieee80211_sub_if_data is internal to mac80211, so not possible to get netdev in this way.
>>
>> any other ideas on how to get netdev?
> 
> Thinking about this some more, it seems like you'd want to send these down to
> firmware immediately so that they'd be available for NS offload. Does firmware
> support NS offload even when host is awake (I think the downstream Android
> driver supports that)?
> 
> So perhaps an alternative approach is to collect the information you need from
> the notification and then schedule a workqueue to actually send the
> information to firmware?
As is pointed out by johannes in another reply, if you are OK with original proposal I can go getting ieee80211_vif_to_wdev(arvif->vif)->netdev in ath12k_wow_prepare_ns_offloa().
Jeff Johnson June 3, 2024, 2:22 p.m. UTC | #6
On 6/2/2024 7:48 PM, Baochen Qiang wrote:
> 
> 
> On 6/1/2024 1:26 AM, Jeff Johnson wrote:
>> On 5/30/2024 10:11 PM, Baochen Qiang wrote:
>>>
>>>
>>> On 5/31/2024 11:42 AM, Baochen Qiang wrote:
>>>>>> +static void ath12k_wow_prepare_ns_offload(struct ath12k_vif *arvif,
>>>>>> +					  struct wmi_arp_ns_offload_arg *offload)
>>>>>> +{
>>>>>> +	struct inet6_dev *idev = arvif->idev;
>>>>> as noted above does it make more sense to get the netdev associated with the
>>>>> arvif and then use in6_dev_get(net_device) to get the inet6_dev rather than
>>>>> caching the pointer from the ipv6_addr_changed() callback?
>>>> Ah.. I didn't note that we can get inet6_dev in such a way, just thought the only way is to cache it in ipv6_changed() callback.
>>>>
>>>> will get it using the following in next version:
>>>> 	struct ieee80211_vif *vif = container_of(arvif)
>>>> 	struct ieee80211_sub_if_data *sub_if_data = container_of(vif)
>>>> 	struct net_dev *ndev = sub_if_data->dev
>>>> 	struct inet6_dev *idev = in6_dev_get(ndev)
>>> Just found that ieee80211_sub_if_data is internal to mac80211, so not possible to get netdev in this way.
>>>
>>> any other ideas on how to get netdev?
>>
>> Thinking about this some more, it seems like you'd want to send these down to
>> firmware immediately so that they'd be available for NS offload. Does firmware
>> support NS offload even when host is awake (I think the downstream Android
>> driver supports that)?
> And really curious about the use case in Android scenario.

It's all about saving power. The application processor consumes way more power
than the firmware processors, so any work that can be performed by firmware
extends battery life.

A laptop running on battery would presumably benefit from this as well.

But let's get basic functionality in place using ieee80211_vif_to_wdev()