Making 10.1.467 based CT firmware do mgt-over-htt

[Michal Kazior]

On 13 July 2015 at 06:36, Ben Greear <greearb at candelatech.com> wrote:
> I have been working on making my CT firmware able to do mgt frames
> over the normal HTT transport instead of over wmi.  This should
> significantly
> improve APs that are trying to deal with stations going out of range.
>
> A fairly trivial change in the firmware lets ath10k in station mode
> associate in both
> OPEN and WPA2 (I did not test further at this time).
>
> But, on the AP side, this same change ends up with probe responses going
> out with 10 bytes missing from the end of the frame.  If I simply do a
> skb_put(foo, 10)
> before sending to firmware, then AP mode works (OPEN, against ath9k AP).
>
> But, this skb_put() breaks the station (OS crashed and rebooted).
>
> I can probably find a particular set of logic that puts or does not put
> the 10 extra bytes accordingly, but I am curious if anyone knows any reason
> that AP mode frames are different.  I did find some code
> in firmware that basically subtracts 10 bytes from length:
> len -= (sizeof(struct ieee80211_frame) - WAL_DE_ETHR_HDR_LEN))
>
> This happens for all native wifi pkts though.  Is there some reason why a
> probe response (and maybe other mgt frames?) would not be built for native
> wifi?
>
> use_frags in htt_tx.c will be TRUE in my case, because htt version is 2.2 in
> my firmware.

Perhaps that is the problem right there? HTT 3.0 doesn't use tx frags
for mgmt frames. I didn't investigate the reason but I recall someone
saying that there are some issues with tx frags for mgmt frames..


>
> My driver patch looks like this currently:
>
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h
> b/drivers/net/wireless/ath/ath10k/core.h
> index c2c1f2d..d291121 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -570,6 +570,7 @@ struct ath10k {
>         DECLARE_BITMAP(fw_features, ATH10K_FW_FEATURE_COUNT);
>
>         bool p2p;
> +       bool all_pkts_htt; /* target has no separate mgmt tx command? */

I don't like this idea.

Perhaps we should instead introduce a new IE to explicitly describe
what command should be used for mgmt tx? We'd have at least 3 values
to express: wmi_mgmt_tx, htt_mgmt_tx, htt_tx.


>
>         struct {
>                 enum ath10k_bus bus;
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 815f7fc..a07ce92 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1932,6 +1932,15 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar,
> struct sk_buff *skb)
>         case HTT_T2H_MSG_TYPE_VERSION_CONF: {
>                 htt->target_version_major = resp->ver_resp.major;
>                 htt->target_version_minor = resp->ver_resp.minor;
> +
> +               if ((htt->target_version_major >= 3) ||
> +                   /* CT firmware with HTT-MGT */
> +                   (htt->target_version_major == 2 &&
> +                    htt->target_version_minor == 2))
> +                       ar->all_pkts_htt = true;
> +               else
> +                       ar->all_pkts_htt = false;
> +

I wouldn't rely on HTT versioning...


Michał