> - u8 p2p_ctwindow;
> - bool p2p_oppps;
> + struct ieee80211_p2p_noa_attr p2p_noa_attr;
Shouldn't you also remove sdata.u.mgd.p2p_noa_index? Actually, maybe
not, since that needs to be -1 in some cases, so n/m.
> - if (params->p2p_opp_ps >= 0) {
> - sdata->vif.bss_conf.p2p_oppps = params->p2p_opp_ps;
> + if (params->p2p_opp_ps > 0) {
> + sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow |= BIT(8);
> + changed |= BSS_CHANGED_P2P_PS;
> + } else if (params->p2p_opp_ps == 0) {
> + sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow &= ~BIT(8);
> changed |= BSS_CHANGED_P2P_PS;
BIT(8) can't be right -- you mean 7?
Maybe also time to introduce a constant for this so drivers can use it
as well?
> if (sdata->vif.p2p) {
> - struct ieee80211_p2p_noa_attr noa;
> int ret;
> -
> + struct ieee80211_p2p_noa_attr noa = {};
> + /* We have to handle here such cases:
> + - p2p_attr disapear
got catch, I guess I never noticed that -- also typo ("disappears" or
"disappeared")
wrong comment style though
> + /* We can memcmp() here in case will find broken GO */
> + if (sdata->u.mgd.p2p_noa_index != noa.index) {
> sdata->u.mgd.p2p_noa_index = noa.index;
> + memcpy(&bss_conf->p2p_noa_attr, &noa, sizeof(noa));
> + changed |= BSS_CHANGED_P2P_PS;
> /*
> * make sure we update all information, the CRC
> * mechanism doesn't look at P2P attributes.
> */
> ifmgd->beacon_crc_valid = false;
> }
> +
> + /* Or different handling:
> +
> + // Version 2:
Are you asking me to pick version? I guess the first one above doesn't
handle the disappearance case very well?
> + int ret;
> + struct ieee80211_p2p_noa_attr noa = {};
> + ret = cfg80211_get_p2p_attr(mgmt->u.beacon.variable,
> + len - baselen,
> + IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
> + (u8 *) &noa, 2);
why 2 now?
> + if (ret <= 0) {
> + if (sdata->u.mgd.p2p_noa_index) {
> + // NOA attr disapear
> + sdata->u.mgd.p2p_noa_index = 0;
> + memset(&bss_conf->p2p_noa_attr, 0,
> + sizeof(bss_conf->p2p_noa_attr));
> + changed |= BSS_CHANGED_P2P_PS;
> + ifmgd->beacon_crc_valid = false;
> + }
> + } else if (sdata->u.mgd.p2p_noa_index != noa.index) {
> + // NOA changed, noa desc(s) could disapear
> + memset(&bss_conf->p2p_noa_attr, 0,
> + sizeof(bss_conf->p2p_noa_attr);
> + ret = cfg80211_get_p2p_attr(mgmt->u.beacon.variable,
> + len - baselen,
> + IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
> + (u8 *) &bss_conf->p2p_noa_attr,
> + sizeof(bss_conf->p2p_noa_attr));
I don't see why you'd want to retrieve it twice? It will only copy as
much as is present and return the data length.
> + // Version 3:
> + int ret;
> + memset(&bss_conf->p2p_noa_attr, 0, sizeof(bss_conf->p2p_noa_attr));
> + ret = cfg80211_get_p2p_attr(mgmt->u.beacon.variable,
> + len - baselen,
> + IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
> + (u8 *) &bss_conf->p2p_noa_attr,
> + sizeof(bss_conf->p2p_noa_attr));
> + if (sdata->u.mgd.p2p_noa_index != noa.index) {
> + sdata->u.mgd.p2p_noa_index = noa.index;
> + changed |= BSS_CHANGED_P2P_PS;
> + ifmgd->beacon_crc_valid = false;
> + }
Here you didn't check "ret", so the "disappeared entirely" case isn't
handled?
Seems like it should be like this:
ret = get_p2p_attr(...)
if (sdata->u.mgd.p2p_noa_index != noa.index || !ret) {
if (ret)
p2p_noa_index = noa.index
else
p2p_noa_index = -1;
changed |= ...
crc_valid = false;
}
to handle all the cases, including dis- and reappearance?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
