| Message ID | 1509409746-26592-1-git-send-email-rmanohar@qti.qualcomm.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
On 10/30/2017 05:29 PM, Rajkumar Manoharan wrote: > > Update last_ack status for all except station probing frames > (i.e null data). Otherwise the station inactivity duration is > cleared whenever AP is checking presence of idle stations by sending > null data frame for every inactive threshold (ap_max_inactivity). > Though the station is idle for longer period, the inactive > time in station dump is restricted ap_max_inactivity threshold. > > Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com> > --- To clarify: wouldn't it break hostapd's "inactivity timeot kick-out" logic in hostapd? As hostapd generates NULL frames to idle stations to reset their inactivity time, otherwise it will disassociate a station, after an ap_max_inactivity period.
> To clarify: wouldn't it break hostapd's "inactivity timeot kick-out" > logic in hostapd? As hostapd generates NULL frames to idle stations to reset > their inactivity time, otherwise it will disassociate a station, after an > ap_max_inactivity period. Igor, I validated hostpad inactivity monitoring with this change and Hostapd is sending Nulldata periodically for inactive stations. Hostapd resets timer based on tx ack status of nulldata. So retaining actual station inactive period will not break hostapd monitoring. Please report if you see any issue with this change. Thanks for your feedback. -Rajkumar
On Mon, 2017-10-30 at 17:29 -0700, Rajkumar Manoharan wrote: > Update last_ack status for all except station probing frames > (i.e null data). Otherwise the station inactivity duration is > cleared whenever AP is checking presence of idle stations by sending > null data frame for every inactive threshold (ap_max_inactivity). You make it sound like this is a problem, but don't explain why it would be? I don't really see it anyway. johannes
> On Mon, 2017-10-30 at 17:29 -0700, Rajkumar Manoharan wrote: > > Update last_ack status for all except station probing frames (i.e null > > data). Otherwise the station inactivity duration is cleared whenever > > AP is checking presence of idle stations by sending null data frame > > for every inactive threshold (ap_max_inactivity). > > You make it sound like this is a problem, but don't explain why it would be? I > don't really see it anyway. > For steering an idle station from one BSS to another, the steering application has to know the actual station idle period. But if the idle period is cleared by ap_max_inactivity, the steering application cannot steer the station even though the station is not sending or receiving any data or mgmt. frame. Though keeping longer idle threshold (ap_max_inactivity) than steering application limit may help to identify idle station, it will differ removing the stations that left BSS. -Rajkumar
On 11/06/2017 09:44 AM, Rajkumar Manoharan wrote: >> On Mon, 2017-10-30 at 17:29 -0700, Rajkumar Manoharan wrote: >>> Update last_ack status for all except station probing frames (i.e null >>> data). Otherwise the station inactivity duration is cleared whenever >>> AP is checking presence of idle stations by sending null data frame >>> for every inactive threshold (ap_max_inactivity). >> >> You make it sound like this is a problem, but don't explain why it would be? I >> don't really see it anyway. >> > For steering an idle station from one BSS to another, the steering application > has to know the actual station idle period. But if the idle period is cleared by > ap_max_inactivity, the steering application cannot steer the station even though > the station is not sending or receiving any data or mgmt. frame. > > Though keeping longer idle threshold (ap_max_inactivity) than steering application > limit may help to identify idle station, it will differ removing the stations that left BSS. > > -Rajkumar > Hi Rajkumar, this will not help in all the cases as some drivers may choose to implement "inactivity" logic in firmware and firmware will send probing frames itself if required, resetting inactivity period. Besides, at this level we do not know who sent NULL frames and for which purpose, it doesn't look like a good idea to just ignore it. I think that for what it was supposed to be, inactivity_ms works as expected and it can be used to identify "dead" stations. What you probably need is something different, called for example "rx_idle_time" that will keep track of how long STA itself did not send any frames (ignoring ACKs).
> >>> Update last_ack status for all except station probing frames (i.e > >>> null data). Otherwise the station inactivity duration is cleared > >>> whenever AP is checking presence of idle stations by sending null > >>> data frame for every inactive threshold (ap_max_inactivity). > >> > >> You make it sound like this is a problem, but don't explain why it > >> would be? I don't really see it anyway. > >> > > For steering an idle station from one BSS to another, the steering > > application has to know the actual station idle period. But if the > > idle period is cleared by ap_max_inactivity, the steering application > > cannot steer the station even though the station is not sending or receiving > any data or mgmt. frame. > > > > Though keeping longer idle threshold (ap_max_inactivity) than steering > > application limit may help to identify idle station, it will differ removing the > stations that left BSS. > > > Hi Rajkumar, this will not help in all the cases as some drivers may choose to > implement "inactivity" logic in firmware and firmware will send probing frames > itself if required, resetting inactivity period. > In offload approach, no need to report tx_ack for firmware generated frames. no? > Besides, at this level we do not know who sent NULL frames and for which > purpose, it doesn't look like a good idea to just ignore it. > I had the same doubt. But AFAIK, probe_client netlink command is used by Hostapd only. That’s why I added comment section before the change. > I think that for what it was supposed to be, inactivity_ms works as expected and > it can be used to identify "dead" stations. What you probably need is something > different, called for example "rx_idle_time" > that will keep track of how long STA itself did not send any frames (ignoring > ACKs). > Hmm.. "rx_idle_time" only not enough to determine active state. Because a station can receive uni-direction data traffic. For example, If the station is connected and idle for half an hour but within BSS, then both "inactivity_ms" and "connected_time" should be 30 mins. Isn't it?. -Rajkumar
On Mon, 2017-11-06 at 20:20 +0000, Rajkumar Manoharan wrote: > > > For steering an idle station from one BSS to another, the steering > > > application has to know the actual station idle period. But if the > > > idle period is cleared by ap_max_inactivity, the steering application > > > cannot steer the station even though the station is not sending or receiving > > > > any data or mgmt. frame. I guess you want something else. Why even consider a station that's sending mgmt frames as non-idle? You can just use the data TX/RX counters to figure out if the station has been idle. You've not convinced me that changing the current logic here makes any sense. johannes
> On Mon, 2017-11-06 at 20:20 +0000, Rajkumar Manoharan wrote: > > > > For steering an idle station from one BSS to another, the steering > > > > application has to know the actual station idle period. But if the > > > > idle period is cleared by ap_max_inactivity, the steering > > > > application cannot steer the station even though the station is > > > > not sending or receiving > > > > > > any data or mgmt. frame. > > I guess you want something else. Why even consider a station that's sending > mgmt frames as non-idle? > > You can just use the data TX/RX counters to figure out if the station has been > idle. > Hmm.. But polling TX/RX counters will not give exact activity time. Moreover It will be useful if STA's last data activity tstamp is logged in station dump. No? > You've not convinced me that changing the current logic here makes any sense. > I thought adding new inactive time attribute may conflict with existing last_active info. As Igor mentioned earlier, what is your thought of adding new attribute "sta_last_data_active" to monitor data frame. Any suggestions are welcome. -Rajkumar
On Wed, 2017-11-15 at 19:21 +0000, Rajkumar Manoharan wrote: > > > Hmm.. But polling TX/RX counters will not give exact activity time. Yeah but if you do it every minute, you'll know if it was idle for roughly 5 minutes, or so... dunno, things could be done. > Moreover > It will be useful if STA's last data activity tstamp is logged in station dump. No? I don't object to adding that, though we should be careful with adding things and see if we _really_ need them, since they all have a cost. johannes
diff --git a/net/mac80211/status.c b/net/mac80211/status.c index da7427a41529..24ce416f74cd 100644 --- a/net/mac80211/status.c +++ b/net/mac80211/status.c @@ -185,10 +185,21 @@ static void ieee80211_check_pending_bar(struct sta_info *sta, u8 *addr, u8 tid) static void ieee80211_frame_acked(struct sta_info *sta, struct sk_buff *skb) { struct ieee80211_mgmt *mgmt = (void *) skb->data; + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct ieee80211_local *local = sta->local; struct ieee80211_sub_if_data *sdata = sta->sdata; - if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) + /* Update last_ack status for all except station probing frames + * (i.e null data). Otherwise the station inactivity duration is cleared + * whenever AP is checking presence of idle stations by sending + * null data frame for every inactive threshold (ap_max_inactivity). + * Though the station is idle for longer period, the inactive time in + * station dump is restricted ap_max_inactivity threshold. + */ + if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) && + !((info->flags & IEEE80211_TX_INTFL_NL80211_FRAME_TX) && + (ieee80211_is_nullfunc(mgmt->frame_control) || + ieee80211_is_qos_nullfunc(mgmt->frame_control)))) sta->status_stats.last_ack = jiffies; if (ieee80211_is_data_qos(mgmt->frame_control)) {
Update last_ack status for all except station probing frames (i.e null data). Otherwise the station inactivity duration is cleared whenever AP is checking presence of idle stations by sending null data frame for every inactive threshold (ap_max_inactivity). Though the station is idle for longer period, the inactive time in station dump is restricted ap_max_inactivity threshold. Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com> --- net/mac80211/status.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)