* [PATCH v2 0/2] cfg80211/mac80211: last ack signal support in station dump @ 2018-06-04 14:11 Balaji Pothunoori 2018-06-04 14:11 ` [PATCH v2 1/2] cfg80211: " Balaji Pothunoori 2018-06-04 14:11 ` [PATCH v2 2/2] mac80211: " Balaji Pothunoori 0 siblings, 2 replies; 8+ messages in thread From: Balaji Pothunoori @ 2018-06-04 14:11 UTC (permalink / raw) To: johannes; +Cc: linux-wireless, Balaji Pothunoori This patch adds "last ack signal" support in station dump if driver reports ack rssi for last tx packet. Balaji Pothunoori (2): cfg80211: last ack signal support in station dump mac80211: last ack signal support in station dump v2: - typo corrected in subject include/uapi/linux/nl80211.h | 14 +++++++------- net/mac80211/sta_info.c | 20 ++++++++++++++------ net/wireless/nl80211.c | 8 ++++---- 3 files changed, 25 insertions(+), 17 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] cfg80211: last ack signal support in station dump 2018-06-04 14:11 [PATCH v2 0/2] cfg80211/mac80211: last ack signal support in station dump Balaji Pothunoori @ 2018-06-04 14:11 ` Balaji Pothunoori 2018-06-15 11:41 ` Johannes Berg 2018-06-04 14:11 ` [PATCH v2 2/2] mac80211: " Balaji Pothunoori 1 sibling, 1 reply; 8+ messages in thread From: Balaji Pothunoori @ 2018-06-04 14:11 UTC (permalink / raw) To: johannes; +Cc: linux-wireless, Balaji Pothunoori This patch adds "last ack signal" support in station dump if driver supports. Signed-off-by: Balaji Pothunoori <bpothuno@codeaurora.org> --- v2: - typo corrected in subject include/uapi/linux/nl80211.h | 14 +++++++------- net/wireless/nl80211.c | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 28b3654..3514bef 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -3000,8 +3000,8 @@ enum nl80211_sta_bss_param { * received from the station (u64, usec) * @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit alignment * @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK frame(u8, dBm) - * @NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG: avg signal strength of (data) - * ACK frame (s8, dBm) + * @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of data or management + * ACK frames(s8, dBm) * @__NL80211_STA_INFO_AFTER_LAST: internal * @NL80211_STA_INFO_MAX: highest possible station info attribute */ @@ -3041,7 +3041,7 @@ enum nl80211_sta_info { NL80211_STA_INFO_RX_DURATION, NL80211_STA_INFO_PAD, NL80211_STA_INFO_ACK_SIGNAL, - NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG, + NL80211_STA_INFO_ACK_SIGNAL_AVG, /* keep last */ __NL80211_STA_INFO_AFTER_LAST, @@ -5128,9 +5128,9 @@ enum nl80211_feature_flags { * "radar detected" event. * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports sending and * receiving control port frames over nl80211 instead of the netdevice. - * @NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT: This Driver support data ack - * rssi if firmware support, this flag is to intimate about ack rssi - * support to nl80211. + * @NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT: This Driver support ack rssi if + * firmware support, this flag is to intimate about ack rssi support + * to nl80211. * @NL80211_EXT_FEATURE_TXQS: Driver supports FQ-CoDel-enabled intermediate * TXQs. * @@ -5165,7 +5165,7 @@ enum nl80211_ext_feature_index { NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN, NL80211_EXT_FEATURE_DFS_OFFLOAD, NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211, - NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT, + NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT, NL80211_EXT_FEATURE_TXQS, /* add new features before the definition below */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 07514ca..29cf5fd 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -4650,11 +4650,11 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid, PUT_SINFO_U64(RX_DROP_MISC, rx_dropped_misc); PUT_SINFO_U64(BEACON_RX, rx_beacon); PUT_SINFO(BEACON_SIGNAL_AVG, rx_beacon_signal_avg, u8); - PUT_SINFO(ACK_SIGNAL, ack_signal, u8); if (wiphy_ext_feature_isset(&rdev->wiphy, - NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT)) - PUT_SINFO(DATA_ACK_SIGNAL_AVG, avg_ack_signal, s8); - + NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT)) { + PUT_SINFO(ACK_SIGNAL, ack_signal, u8); + PUT_SINFO(ACK_SIGNAL_AVG, avg_ack_signal, s8); + } #undef PUT_SINFO #undef PUT_SINFO_U64 -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] cfg80211: last ack signal support in station dump 2018-06-04 14:11 ` [PATCH v2 1/2] cfg80211: " Balaji Pothunoori @ 2018-06-15 11:41 ` Johannes Berg 2018-06-18 9:18 ` Balaji Pothunoori 0 siblings, 1 reply; 8+ messages in thread From: Johannes Berg @ 2018-06-15 11:41 UTC (permalink / raw) To: Balaji Pothunoori; +Cc: linux-wireless On Mon, 2018-06-04 at 19:41 +0530, Balaji Pothunoori wrote: > > +++ b/include/uapi/linux/nl80211.h > @@ -3000,8 +3000,8 @@ enum nl80211_sta_bss_param { > * received from the station (u64, usec) > * @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit alignment > * @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK frame(u8, dBm) > - * @NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG: avg signal strength of (data) > - * ACK frame (s8, dBm) > + * @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of data or management > + * ACK frames(s8, dBm) > * @__NL80211_STA_INFO_AFTER_LAST: internal > * @NL80211_STA_INFO_MAX: highest possible station info attribute > */ > @@ -3041,7 +3041,7 @@ enum nl80211_sta_info { > NL80211_STA_INFO_RX_DURATION, > NL80211_STA_INFO_PAD, > NL80211_STA_INFO_ACK_SIGNAL, > - NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG, > + NL80211_STA_INFO_ACK_SIGNAL_AVG, Wait, what happened here? You can't remove old API. > @@ -5128,9 +5128,9 @@ enum nl80211_feature_flags { > * "radar detected" event. > * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports sending and > * receiving control port frames over nl80211 instead of the netdevice. > - * @NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT: This Driver support data ack > - * rssi if firmware support, this flag is to intimate about ack rssi > - * support to nl80211. > + * @NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT: This Driver support ack rssi if > + * firmware support, this flag is to intimate about ack rssi support > + * to nl80211. Same here, why are you removing the data-ack-signal API? Is that just a rebase error or something? Or is it intentional, but then please explain what you're trying to do and I can help find a correct solutions. johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] cfg80211: last ack signal support in station dump 2018-06-15 11:41 ` Johannes Berg @ 2018-06-18 9:18 ` Balaji Pothunoori 2018-06-18 20:51 ` Johannes Berg 0 siblings, 1 reply; 8+ messages in thread From: Balaji Pothunoori @ 2018-06-18 9:18 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless On 2018-06-15 17:11, Johannes Berg wrote: > On Mon, 2018-06-04 at 19:41 +0530, Balaji Pothunoori wrote: >> >> +++ b/include/uapi/linux/nl80211.h >> @@ -3000,8 +3000,8 @@ enum nl80211_sta_bss_param { >> * received from the station (u64, usec) >> * @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit >> alignment >> * @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK >> frame(u8, dBm) >> - * @NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG: avg signal strength of >> (data) >> - * ACK frame (s8, dBm) >> + * @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of data or >> management >> + * ACK frames(s8, dBm) >> * @__NL80211_STA_INFO_AFTER_LAST: internal >> * @NL80211_STA_INFO_MAX: highest possible station info attribute >> */ >> @@ -3041,7 +3041,7 @@ enum nl80211_sta_info { >> NL80211_STA_INFO_RX_DURATION, >> NL80211_STA_INFO_PAD, >> NL80211_STA_INFO_ACK_SIGNAL, >> - NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG, >> + NL80211_STA_INFO_ACK_SIGNAL_AVG, > > Wait, what happened here? You can't remove old API. Here is my intention is to make the unique average ack signal and last ack signal support in station dump irrespective of data or management tx ack packet. Do you want me to add a new API for management tx ack packet? > >> @@ -5128,9 +5128,9 @@ enum nl80211_feature_flags { >> * "radar detected" event. >> * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports >> sending and >> * receiving control port frames over nl80211 instead of the >> netdevice. >> - * @NL80211_EXT_FEATURE_DATA_ACK_SIGNAL_SUPPORT: This Driver support >> data ack >> - * rssi if firmware support, this flag is to intimate about ack rssi >> - * support to nl80211. >> + * @NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT: This Driver support ack >> rssi if >> + * firmware support, this flag is to intimate about ack rssi support >> + * to nl80211. > > Same here, why are you removing the data-ack-signal API? > > Is that just a rebase error or something? > > Or is it intentional, but then please explain what you're trying to do > and I can help find a correct solutions. > > johannes Here i renamed above API because in driver i will fill this "NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT" if firmware supports either of the one "WMI_SERVICE_TX_DATA_ACK_RSSI" or "WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS". This is to make unique avg ack signal/last ack signal support. Regards, Balaji. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] cfg80211: last ack signal support in station dump 2018-06-18 9:18 ` Balaji Pothunoori @ 2018-06-18 20:51 ` Johannes Berg 0 siblings, 0 replies; 8+ messages in thread From: Johannes Berg @ 2018-06-18 20:51 UTC (permalink / raw) To: Balaji Pothunoori; +Cc: linux-wireless On Mon, 2018-06-18 at 14:48 +0530, Balaji Pothunoori wrote: > On 2018-06-15 17:11, Johannes Berg wrote: > > On Mon, 2018-06-04 at 19:41 +0530, Balaji Pothunoori wrote: > > > > > > +++ b/include/uapi/linux/nl80211.h > > > @@ -3000,8 +3000,8 @@ enum nl80211_sta_bss_param { > > > * received from the station (u64, usec) > > > * @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit > > > alignment > > > * @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK > > > frame(u8, dBm) > > > - * @NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG: avg signal strength of > > > (data) > > > - * ACK frame (s8, dBm) > > > + * @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of data or > > > management > > > + * ACK frames(s8, dBm) > > > * @__NL80211_STA_INFO_AFTER_LAST: internal > > > * @NL80211_STA_INFO_MAX: highest possible station info attribute > > > */ > > > @@ -3041,7 +3041,7 @@ enum nl80211_sta_info { > > > NL80211_STA_INFO_RX_DURATION, > > > NL80211_STA_INFO_PAD, > > > NL80211_STA_INFO_ACK_SIGNAL, > > > - NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG, > > > + NL80211_STA_INFO_ACK_SIGNAL_AVG, > > > > Wait, what happened here? You can't remove old API. > > Here is my intention is to make the unique average ack signal and last > ack signal > support in station dump irrespective of data or management tx ack > packet. > Do you want me to add a new API for management tx ack packet? Well, you can't remove old API. And the data-ACK was explicitly added because of concerns about signal varying a lot with MCS, so ACK signal is more reliable. I suppose the original use here didn't really *need* just *data* ACK, so perhaps we can redefine it - you'd know better? After all, you defined the old API :-)) But still, you can't just remove it. I think we're probably OK to redefine it but then you need to keep API compatibility by adding the necessary defines. johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] mac80211: last ack signal support in station dump 2018-06-04 14:11 [PATCH v2 0/2] cfg80211/mac80211: last ack signal support in station dump Balaji Pothunoori 2018-06-04 14:11 ` [PATCH v2 1/2] cfg80211: " Balaji Pothunoori @ 2018-06-04 14:11 ` Balaji Pothunoori 2018-06-15 11:43 ` Johannes Berg 1 sibling, 1 reply; 8+ messages in thread From: Balaji Pothunoori @ 2018-06-04 14:11 UTC (permalink / raw) To: johannes; +Cc: linux-wireless, Balaji Pothunoori This patch adds "last ack signal" and "avg ack signal" support in station dump for valid ack rssi. Signed-off-by: Balaji Pothunoori <bpothuno@codeaurora.org> --- v2: - typo corrected in subject net/mac80211/sta_info.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 6428f1a..12b618e 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -2310,13 +2310,21 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo, sinfo->filled |= BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL); } - if (ieee80211_hw_check(&sta->local->hw, REPORTS_TX_ACK_STATUS) && - !(sinfo->filled & BIT_ULL(NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG))) { - sinfo->avg_ack_signal = - -(s8)ewma_avg_signal_read( + if (ieee80211_hw_check(&sta->local->hw, REPORTS_TX_ACK_STATUS)) { + if (sta->status_stats.ack_signal_filled && ((!(sinfo->filled & + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL))) || + (!(sinfo->filled & + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG))))) { + sinfo->ack_signal = + sta->status_stats.last_ack_signal; + sinfo->filled |= + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL); + sinfo->avg_ack_signal = + -(s8)ewma_avg_signal_read( &sta->status_stats.avg_ack_signal); - sinfo->filled |= - BIT_ULL(NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG); + sinfo->filled |= + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG); + } } } -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mac80211: last ack signal support in station dump 2018-06-04 14:11 ` [PATCH v2 2/2] mac80211: " Balaji Pothunoori @ 2018-06-15 11:43 ` Johannes Berg 2018-06-18 10:25 ` Balaji Pothunoori 0 siblings, 1 reply; 8+ messages in thread From: Johannes Berg @ 2018-06-15 11:43 UTC (permalink / raw) To: Balaji Pothunoori; +Cc: linux-wireless On Mon, 2018-06-04 at 19:41 +0530, Balaji Pothunoori wrote: > > + if (ieee80211_hw_check(&sta->local->hw, REPORTS_TX_ACK_STATUS)) { > + if (sta->status_stats.ack_signal_filled && ((!(sinfo->filled & > + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL))) || > + (!(sinfo->filled & > + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG))))) { Uh, wow, the indentation here is awful - please break with && and || on the end of line and indent properly according to the nesting level. If a line ends up being longer than 80, I think that's better than this monster expression :) > + sinfo->ack_signal = > + sta->status_stats.last_ack_signal; > + sinfo->filled |= > + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL); > + sinfo->avg_ack_signal = > + -(s8)ewma_avg_signal_read( > &sta->status_stats.avg_ack_signal); > - sinfo->filled |= > - BIT_ULL(NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG); > + sinfo->filled |= > + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG); > + } Clearly your previous patch would even break the kernel compile since the DATA_ACK_SIGNAL_AVG is still used here. Finally, please also explain why you're adding this feature, at least in the cover letter ("PATCH 0/2"), I can reuse that as a merge commit message if necessary. johannes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] mac80211: last ack signal support in station dump 2018-06-15 11:43 ` Johannes Berg @ 2018-06-18 10:25 ` Balaji Pothunoori 0 siblings, 0 replies; 8+ messages in thread From: Balaji Pothunoori @ 2018-06-18 10:25 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless On 2018-06-15 17:13, Johannes Berg wrote: > On Mon, 2018-06-04 at 19:41 +0530, Balaji Pothunoori wrote: >> >> + if (ieee80211_hw_check(&sta->local->hw, REPORTS_TX_ACK_STATUS)) { >> + if (sta->status_stats.ack_signal_filled && ((!(sinfo->filled & >> + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL))) || >> + (!(sinfo->filled & >> + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG))))) { > > Uh, wow, the indentation here is awful - please break with && and || on > the end of line and indent properly according to the nesting level. > > If a line ends up being longer than 80, I think that's better than this > monster expression :) Sure , i will modify in v3. > >> + sinfo->ack_signal = >> + sta->status_stats.last_ack_signal; >> + sinfo->filled |= >> + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL); >> + sinfo->avg_ack_signal = >> + -(s8)ewma_avg_signal_read( >> &sta->status_stats.avg_ack_signal); >> - sinfo->filled |= >> - BIT_ULL(NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG); >> + sinfo->filled |= >> + BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG); >> + } > > Clearly your previous patch would even break the kernel compile since > the DATA_ACK_SIGNAL_AVG is still used here. Here i have used "NL80211_STA_INFO_ACK_SIGNAL_AVG" not as "NL80211_STA_INFO_DATA_ACK_SIGNAL_AVG". > > > > Finally, please also explain why you're adding this feature, at least > in > the cover letter ("PATCH 0/2"), I can reuse that as a merge commit > message if necessary. Sure, i will update in v3. > > johannes Regards, Balaji. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-18 20:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-04 14:11 [PATCH v2 0/2] cfg80211/mac80211: last ack signal support in station dump Balaji Pothunoori 2018-06-04 14:11 ` [PATCH v2 1/2] cfg80211: " Balaji Pothunoori 2018-06-15 11:41 ` Johannes Berg 2018-06-18 9:18 ` Balaji Pothunoori 2018-06-18 20:51 ` Johannes Berg 2018-06-04 14:11 ` [PATCH v2 2/2] mac80211: " Balaji Pothunoori 2018-06-15 11:43 ` Johannes Berg 2018-06-18 10:25 ` Balaji Pothunoori
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.