2013/3/18 Johannes Berg <johannes@xxxxxxxxxxxxxxxx>:
>
>> - 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?
>
Added changes. Seems sdata->u.mgd.p2p_noa_index is required while
noa.index valid range is 0-255 and decide base on index.
I think code cover now all cases.
---
include/net/mac80211.h | 3 +--
net/mac80211/cfg.c | 19 ++++++++++++++-----
net/mac80211/ieee80211_i.h | 2 +-
net/mac80211/mlme.c | 42 ++++++++++++++++++++++++++----------------
net/mac80211/trace.h | 6 ++----
5 files changed, 44 insertions(+), 28 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cdd7cea..582e743 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -363,8 +363,7 @@ struct ieee80211_bss_conf {
size_t ssid_len;
bool hidden_ssid;
int txpower;
- u8 p2p_ctwindow;
- bool p2p_oppps;
+ struct ieee80211_p2p_noa_attr p2p_noa_attr;
};
/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1d1ddab..6dc592b 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -960,8 +960,12 @@ static int ieee80211_start_ap(struct wiphy
*wiphy, struct net_device *dev,
sdata->vif.bss_conf.hidden_ssid =
(params->hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE);
- sdata->vif.bss_conf.p2p_ctwindow = params->p2p_ctwindow;
- sdata->vif.bss_conf.p2p_oppps = params->p2p_opp_ps;
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow =
+ params->p2p_ctwindow & 0x7F;
+ if (params->p2p_opp_ps)
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow |= BIT(7);
+ else
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow &= ~BIT(7);
err = ieee80211_assign_beacon(sdata, ¶ms->beacon);
if (err < 0)
@@ -1956,12 +1960,17 @@ static int ieee80211_change_bss(struct wiphy *wiphy,
}
if (params->p2p_ctwindow >= 0) {
- sdata->vif.bss_conf.p2p_ctwindow = params->p2p_ctwindow;
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow &= ~0x7f;
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow |=
+ params->p2p_ctwindow & 0x7f;
changed |= BSS_CHANGED_P2P_PS;
}
- 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(7);
+ changed |= BSS_CHANGED_P2P_PS;
+ } else if (params->p2p_opp_ps == 0) {
+ sdata->vif.bss_conf.p2p_noa_attr.oppps_ctwindow &= ~BIT(7);
changed |= BSS_CHANGED_P2P_PS;
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f4433f0..11451ee 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -442,7 +442,7 @@ struct ieee80211_if_managed {
u8 use_4addr;
- u8 p2p_noa_index;
+ s16 p2p_noa_index;
/* Signal strength from the last Beacon frame in the current BSS. */
int last_beacon_signal;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4fecfb8..252f4a0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1655,18 +1655,17 @@ static void ieee80211_set_associated(struct
ieee80211_sub_if_data *sdata,
rcu_read_lock();
ies = rcu_dereference(cbss->ies);
if (ies) {
- struct ieee80211_p2p_noa_attr noa;
int ret;
ret = cfg80211_get_p2p_attr(
ies->data, ies->len,
IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
- (u8 *) &noa, sizeof(noa));
+ (u8 *) &bss_conf->p2p_noa_attr,
+ sizeof(bss_conf->p2p_noa_attr));
if (ret >= 2) {
- bss_conf->p2p_oppps = noa.oppps_ctwindow & 0x80;
- bss_conf->p2p_ctwindow = noa.oppps_ctwindow & 0x7f;
+ sdata->u.mgd.p2p_noa_index =
+ bss_conf->p2p_noa_attr.index;
bss_info_changed |= BSS_CHANGED_P2P_PS;
- sdata->u.mgd.p2p_noa_index = noa.index;
}
}
rcu_read_unlock();
@@ -1791,8 +1790,9 @@ static void ieee80211_set_disassoc(struct
ieee80211_sub_if_data *sdata,
changed |= BSS_CHANGED_ASSOC;
sdata->vif.bss_conf.assoc = false;
- sdata->vif.bss_conf.p2p_ctwindow = 0;
- sdata->vif.bss_conf.p2p_oppps = false;
+ ifmgd->p2p_noa_index = -1;
+ memset(&sdata->vif.bss_conf.p2p_noa_attr, 0,
+ sizeof(sdata->vif.bss_conf.p2p_noa_attr));
/* on the next assoc, re-program HT/VHT parameters */
memset(&ifmgd->ht_capa, 0, sizeof(ifmgd->ht_capa));
@@ -2953,22 +2953,31 @@ ieee80211_rx_mgmt_beacon(struct
ieee80211_sub_if_data *sdata,
}
if (sdata->vif.p2p) {
- struct ieee80211_p2p_noa_attr noa;
+ struct ieee80211_p2p_noa_attr noa = {};
int ret;
ret = cfg80211_get_p2p_attr(mgmt->u.beacon.variable,
len - baselen,
IEEE80211_P2P_ATTR_ABSENCE_NOTICE,
(u8 *) &noa, sizeof(noa));
- if (ret >= 2 && sdata->u.mgd.p2p_noa_index != noa.index) {
- bss_conf->p2p_oppps = noa.oppps_ctwindow & 0x80;
- bss_conf->p2p_ctwindow = noa.oppps_ctwindow & 0x7f;
+
+ if (ret >= 2) {
+ if (sdata->u.mgd.p2p_noa_index != noa.index) {
+ /* valid noa_attr and index changed */
+ 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;
+ }
+ } else if (sdata->u.mgd.p2p_noa_index != -1) {
+ /* noa_attr not found and we had valid noa_attr before */
+ sdata->u.mgd.p2p_noa_index = -1;
+ memset(&bss_conf->p2p_noa_attr, 0, sizeof(bss_conf->p2p_noa_attr));
changed |= BSS_CHANGED_P2P_PS;
- sdata->u.mgd.p2p_noa_index = noa.index;
- /*
- * make sure we update all information, the CRC
- * mechanism doesn't look at P2P attributes.
- */
ifmgd->beacon_crc_valid = false;
}
}
@@ -3511,6 +3520,7 @@ void ieee80211_sta_setup_sdata(struct
ieee80211_sub_if_data *sdata)
ifmgd->powersave = sdata->wdev.ps;
ifmgd->uapsd_queues = IEEE80211_DEFAULT_UAPSD_QUEUES;
ifmgd->uapsd_max_sp_len = IEEE80211_DEFAULT_MAX_SP_LEN;
+ ifmgd->p2p_noa_index = -1;
mutex_init(&ifmgd->mtx);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index e7db2b8..b9322b5 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -359,8 +359,7 @@ TRACE_EVENT(drv_bss_info_changed,
__dynamic_array(u8, ssid, info->ssid_len);
__field(bool, hidden_ssid);
__field(int, txpower)
- __field(u8, p2p_ctwindow)
- __field(bool, p2p_oppps)
+ __field(u8, p2p_oppps_ctwindow)
),
TP_fast_assign(
@@ -400,8 +399,7 @@ TRACE_EVENT(drv_bss_info_changed,
memcpy(__get_dynamic_array(ssid), info->ssid, info->ssid_len);
__entry->hidden_ssid = info->hidden_ssid;
__entry->txpower = info->txpower;
- __entry->p2p_ctwindow = info->p2p_ctwindow;
- __entry->p2p_oppps = info->p2p_oppps;
+ __entry->p2p_oppps_ctwindow = info->p2p_noa_attr.oppps_ctwindow;
),
TP_printk(
--
1.7.9.5
--
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
