On Wed, Jun 12, 2019 at 10:31:52PM -0700, Miles Hu wrote:
> Cross check between firmware and hostapd config capabilities.
> diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c
> static int ieee80211ax_supported_he_capab(struct hostapd_iface *iface)
> + u32 cap_ulmimo = 0, cap_ulofdma = 0;
What are those for? They seem to be unused and generate compiler
warnings.
> +#define HE_CAP_CHECK2(hw_cap, cap, bytes, cap1, bytes1, conf) \
> + do { \
> + if (conf && !(_ieee80211he_cap_check(hw_cap, bytes, cap) || \
> + _ieee80211he_cap_check(hw_cap, bytes1, cap1))) { \
> + wpa_printf(MSG_ERROR, "Driver does not support configured HE" \
> + " capability [%s-%s]", #cap, #cap1); \
> + return 0; \
> + } \
> + } while (0)
What exactly is this trying to do? !(cap || cap2) is identical to !cap
&& !cap2. Is that what this two-capability-version is trying to do?
Check that the driver does not support neither of these, i.e., it is
fine if either is supported? That error message is quite confusing if
that is the case..
> + HE_CAP_CHECK2(hw->phy_cap, HE_PHYCAP_UL_MUMIMO_CAPB,
> + HE_PHYCAP_UL_MUMIMO_CAPB_IDX, HE_PHYCAP_UL_MUOFDMA_CAPB,
> + HE_PHYCAP_UL_MUOFDMA_CAPB_IDX, conf->he_mu_edca.he_qos_info);
I'm not sure how to interpret this since he_qos_info includes
information from four undocumented parameters:
he_mu_edca_qos_info_param_count, he_mu_edca_qos_info_q_ack,
he_mu_edca_qos_info_queue_request, and he_mu_edca_qos_info_txop_request.
And it is not obvious how those map to the two PHY capabilities.
> + HE_CAP_CHECK2(hw->mac_cap, HE_MACCAP_TWT_REQUESTER, HE_MACCAP_TWT_REQUESTER_IDX,
> + HE_MACCAP_TWT_RESPONDER, HE_MACCAP_TWT_RESPONDER_IDX,
> + conf->he_op.he_twt_required);
What about this one? conf->he_op.he_twt_required is either 0 or 1, but
HE_CAP_CHECK2() uses two capability bits. Is this trying to check that
the driver support at least TWT requester or TWT responder capability?
These would really need comments to document what the macros do and what
exactly these HE_CAP_CHECK2 cases are validating.
In addition, these checks are breaking mac80211_hwsim test cases
he_params, he_40, and he_80_params. That may be expected since
mac80211_hwsim does not advertise all HE capabilities. However, I'd like
to get a patch that fixes those test cases before the patch to add
validation steps are added.
> diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
>
> /* HE Capabilities Information defines */
>
> -#define HE_MACCAP_TWT_RESPONDER ((u8) BIT(3))
That line does not exist in hostap.git, so it cannot be removed either..
I rejected a patch proposing this because the specified value does not
seem to match P802.11ax.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
