Re: [PATCH] wifi: ath10k: add support to allow broadcast action from RX

[James Prestwood <prestwoj@gmail.com>]

On 11/14/23 12:20 AM, Kalle Valo wrote:
> James Prestwood <prestwoj@gmail.com> writes:
> 
>> On 11/13/23 7:50 AM, Kalle Valo wrote:
>>> James Prestwood <prestwoj@gmail.com> wrote:
>>>
>>>> Advertise support for multicast frame registration and update the RX
>>>> filter with FIF_MCAST_ACTION to allow broadcast action frames to be
>>>> received. Broadcast action frames are needed for the Device
>>>> Provisioning Protocol (DPP) for Presence and PKEX Exchange requests.
>>>>
>>>> Signed-off-by: James Prestwood <prestwoj@gmail.com>
>>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>>> On what hardware and firmware did you test with this? As there's so
>>> many
>>> different combinations in ath10k we use Tested-on tag to document that.
>>> https://wireless.wiki.kernel.org/en/users/drivers/ath10k/submittingpatches#tested-on_tag
>>> As ath10k hardware and firmware can work very differently from each
>>> other I'm
>>> suspicious if this feature really work in all of them.
>>
>> I only tested on a QCA6174 (and I'll add Tested-on for that). This
>> makes sense and maybe enabling unconditionally for all ath10k hardware
>> is the wrong way to go about it.
>>
>> Since I don't have the ability to test every hardware combination
>> hopefully someone from atheros can chime in.
> 
> Heh, Atheros is long gone. But your comment made me remember the good
> old times and smile :)

Oh yeah, QCOM bought Atheros just before I got my first job there. 
Wasn't sure if the remaining devs still thought of themselves "Atheros" 
employees to this day :)

> 
>> Is there some firmware/driver value that can be queried which tells me
>> if broadcast RX is supported?
> 
> A good question for which I don't have an answer. Does anyone else know?
> 
> Do you have a simple test case for this? It would help if people could
> test this feature on their ath10k devices and send us results.

I could try and come up with something. I've been testing with 2 
devices, running the full DPP protocol between... not exactly "simple".

I suppose you could create a station device, register for beacons, just 
sit and see if you see any. I'm not sure though if this is a 1:1 test 
since really its action frame I'm after, and the firmware may treat them 
differently.

> 
>> Or if not is checking ar->hw_rev == ATH10K_HW_QCA6174 good enough?
> 
> BTW instead of checking ar->hw_rev our preference is to add a new
> boolean to struct ath10k_hw_params. That way it's easier to enable and
> disable the feature per hardware version.
> 
>> Or are there sub-variants that may or may not support this?
> 
> There are several QCA6174 variants and you can check the variants from
> ath10k_hw_params_list. For example, hw2.1 or SDIO firmware may very well
> behave different from the PCI firmware. To be on the safe side I think it's
> best to enable the feature only on the hardware versions we have
> verified to work.

Sounds good, I can make it specific to just my hardware and others could 
expand in the future if they need. Out of curiosity is ath9k much more 
limited on unique hardware? I based this patch off one from Jouni for 
ath9k [1] and it unconditionally enables it for the entire driver.

[1] 
https://lore.kernel.org/linux-wireless/20200426084733.7889-1-jouni@codeaurora.org/

Thanks,
James

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k