Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH-v3 1/2] wireless:  Support assoc-at-ms timer in sta-info
@ 2019-04-15 17:21 greearb
  2019-04-15 17:21 ` [PATCH-v3 2/2] mac80211: add assoc-at-ms support greearb
  2019-06-14 13:38 ` [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info Johannes Berg
  0 siblings, 2 replies; 8+ messages in thread
From: greearb @ 2019-04-15 17:21 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

Report time stamp of when sta became associated.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 include/net/cfg80211.h       | 2 ++
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/nl80211.c       | 1 +
 3 files changed, 5 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f49eb1464b7a..a3ad78b9d9f4 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1268,6 +1268,7 @@ struct cfg80211_tid_stats {
  *	indicate the relevant values in this struct for them
  * @connected_time: time(in secs) since a station is last connected
  * @inactive_time: time since last station activity (tx/rx) in milliseconds
+ * @assoc_at_ms: time in ms of the last association
  * @rx_bytes: bytes (size of MPDUs) received from this station
  * @tx_bytes: bytes (size of MPDUs) transmitted to this station
  * @llid: mesh local link id
@@ -1324,6 +1325,7 @@ struct station_info {
 	u64 filled;
 	u32 connected_time;
 	u32 inactive_time;
+	u64 assoc_at_ms;
 	u64 rx_bytes;
 	u64 tx_bytes;
 	u16 llid;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 0f03cfcda965..0fcedde890e7 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3079,6 +3079,7 @@ enum nl80211_sta_bss_param {
  * @NL80211_STA_INFO_TX_DURATION: aggregate PPDU duration for all frames
  *	sent to the station (u64, usec)
  * @NL80211_STA_INFO_AIRTIME_WEIGHT: current airtime weight for station (u16)
+ * @NL80211_STA_INFO_ASSOC_AT_MS: Timestamp of last association
  * @__NL80211_STA_INFO_AFTER_LAST: internal
  * @NL80211_STA_INFO_MAX: highest possible station info attribute
  */
@@ -3124,6 +3125,7 @@ enum nl80211_sta_info {
 	NL80211_STA_INFO_CONNECTED_TO_GATE,
 	NL80211_STA_INFO_TX_DURATION,
 	NL80211_STA_INFO_AIRTIME_WEIGHT,
+	NL80211_STA_INFO_ASSOC_AT_MS,
 
 	/* keep last */
 	__NL80211_STA_INFO_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bcb432815c64..cb5acf026900 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4763,6 +4763,7 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 
 	PUT_SINFO(CONNECTED_TIME, connected_time, u32);
 	PUT_SINFO(INACTIVE_TIME, inactive_time, u32);
+	PUT_SINFO_U64(ASSOC_AT_MS, assoc_at_ms);
 
 	if (sinfo->filled & (BIT_ULL(NL80211_STA_INFO_RX_BYTES) |
 			     BIT_ULL(NL80211_STA_INFO_RX_BYTES64)) &&
-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH-v3 2/2] mac80211:  add assoc-at-ms support.
  2019-04-15 17:21 [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info greearb
@ 2019-04-15 17:21 ` greearb
  2019-06-12 22:53   ` Ben Greear
  2019-06-14 13:38 ` [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info Johannes Berg
  1 sibling, 1 reply; 8+ messages in thread
From: greearb @ 2019-04-15 17:21 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

Report when sta becomes associated.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/sta_info.c | 3 +++
 net/mac80211/sta_info.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 044120c45950..918ca9475fec 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1968,6 +1968,7 @@ int sta_info_move_state(struct sta_info *sta,
 	case IEEE80211_STA_ASSOC:
 		if (sta->sta_state == IEEE80211_STA_AUTH) {
 			set_bit(WLAN_STA_ASSOC, &sta->_flags);
+			sta->assoc_at_ms = ktime_to_ms(ktime_get_real());
 			ieee80211_recalc_min_chandef(sta->sdata);
 			if (!sta->sta.support_p2p_ps)
 				ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
@@ -2197,6 +2198,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 			 BIT_ULL(NL80211_STA_INFO_STA_FLAGS) |
 			 BIT_ULL(NL80211_STA_INFO_BSS_PARAM) |
 			 BIT_ULL(NL80211_STA_INFO_CONNECTED_TIME) |
+			 BIT_ULL(NL80211_STA_INFO_ASSOC_AT_MS) |
 			 BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC);
 
 	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
@@ -2205,6 +2207,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 	}
 
 	sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
+	sinfo->assoc_at_ms = sta->assoc_at_ms;
 	sinfo->inactive_time =
 		jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
 
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index c5d557032be5..e94b942cd564 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -455,6 +455,7 @@ struct ieee80211_sta_rx_stats {
  *	the station when it leaves powersave or polls for frames
  * @driver_buffered_tids: bitmap of TIDs the driver has data buffered on
  * @txq_buffered_tids: bitmap of TIDs that mac80211 has txq data buffered on
+ * @assoc_at_ms: time (in ms) of last association
  * @last_connected: time (in seconds) when a station got connected
  * @last_seq_ctrl: last received seq/frag number from this STA (per TID
  *	plus one for non-QoS frames)
@@ -531,6 +532,7 @@ struct sta_info {
 	unsigned long driver_buffered_tids;
 	unsigned long txq_buffered_tids;
 
+	unsigned long assoc_at_ms;
 	long last_connected;
 
 	/* Updated from RX path only, no locking requirements */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 2/2] mac80211: add assoc-at-ms support.
  2019-04-15 17:21 ` [PATCH-v3 2/2] mac80211: add assoc-at-ms support greearb
@ 2019-06-12 22:53   ` Ben Greear
  0 siblings, 0 replies; 8+ messages in thread
From: Ben Greear @ 2019-06-12 22:53 UTC (permalink / raw)
  To: linux-wireless

On 4/15/19 10:21 AM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Report when sta becomes associated.

While rebasing, I notice this file is just sitting in patchwork.

Any chance this patch and the previous one in the series can be accepted?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 1/2] wireless:  Support assoc-at-ms timer in sta-info
  2019-04-15 17:21 [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info greearb
  2019-04-15 17:21 ` [PATCH-v3 2/2] mac80211: add assoc-at-ms support greearb
@ 2019-06-14 13:38 ` Johannes Berg
  2019-06-14 14:46   ` Ben Greear
  1 sibling, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2019-06-14 13:38 UTC (permalink / raw)
  To: greearb, linux-wireless

On Mon, 2019-04-15 at 10:21 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Report time stamp of when sta became associated.

I guess that makes sense.

> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>  include/net/cfg80211.h       | 2 ++
>  include/uapi/linux/nl80211.h | 2 ++
>  net/wireless/nl80211.c       | 1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index f49eb1464b7a..a3ad78b9d9f4 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1268,6 +1268,7 @@ struct cfg80211_tid_stats {
>   *	indicate the relevant values in this struct for them
>   * @connected_time: time(in secs) since a station is last connected
>   * @inactive_time: time since last station activity (tx/rx) in milliseconds
> + * @assoc_at_ms: time in ms of the last association

I think the "_at_ms" doesn't really make sense. "time in ms" also
doesn't make sense as documentation. I think you need to say what should
be assigned here.

Also, you're actually using ktime_get_real() for this, which again
doesn't make much sense. For exposing an absolute time, I think we're
better off with CLOCK_BOOTTIME like we use elsewhere:

 * @boottime_ns: CLOCK_BOOTTIME timestamp the frame was received at, this is
 *      needed only for beacons and probe responses that update the scan cache.


> + * @NL80211_STA_INFO_ASSOC_AT_MS: Timestamp of last association

_ASSOC_TIMESTAMP or something would make more sense too as the attribute
name, and clearly the documentation needs to spell out the semantics
here too.

johannes


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info
  2019-06-14 13:38 ` [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info Johannes Berg
@ 2019-06-14 14:46   ` Ben Greear
  2019-06-14 14:56     ` Johannes Berg
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Greear @ 2019-06-14 14:46 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless

On 6/14/19 6:38 AM, Johannes Berg wrote:
> On Mon, 2019-04-15 at 10:21 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Report time stamp of when sta became associated.
> 
> I guess that makes sense.
> 
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>   include/net/cfg80211.h       | 2 ++
>>   include/uapi/linux/nl80211.h | 2 ++
>>   net/wireless/nl80211.c       | 1 +
>>   3 files changed, 5 insertions(+)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index f49eb1464b7a..a3ad78b9d9f4 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -1268,6 +1268,7 @@ struct cfg80211_tid_stats {
>>    *	indicate the relevant values in this struct for them
>>    * @connected_time: time(in secs) since a station is last connected
>>    * @inactive_time: time since last station activity (tx/rx) in milliseconds
>> + * @assoc_at_ms: time in ms of the last association
> 
> I think the "_at_ms" doesn't really make sense. "time in ms" also
> doesn't make sense as documentation. I think you need to say what should
> be assigned here.
> 
> Also, you're actually using ktime_get_real() for this, which again
> doesn't make much sense. For exposing an absolute time, I think we're
> better off with CLOCK_BOOTTIME like we use elsewhere:

The point of my patch was to allow 'iw' to return a more precise time
that the station has been associated, so I am not sure that BOOTIME is
a good thing to use for that?

Here are the pertinent parts of my iw patches:


diff --git a/station.c b/station.c
index 25cbbc3..e7738cc 100644
--- a/station.c
+++ b/station.c
@@ -314,6 +314,12 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
                 [NL80211_STA_INFO_ACK_SIGNAL_AVG] = { .type = NLA_U8 },
         };
         char *chain;
+       struct timeval now;
+       unsigned long long now_ms;
+
+       gettimeofday(&now, NULL);
+       now_ms = now.tv_sec * 1000;
+       now_ms += (now.tv_usec / 1000);

         nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
                   genlmsg_attrlen(gnlh, 0), NULL);
@@ -557,8 +563,11 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
         if (sinfo[NL80211_STA_INFO_CONNECTED_TIME])
                 printf("\n\tconnected time:\t%u seconds",
                         nla_get_u32(sinfo[NL80211_STA_INFO_CONNECTED_TIME]));
+       if (sinfo[NL80211_STA_INFO_ASSOC_AT_MS])
+               printf("\n\tassociated at:\t%llu ms",
+                        (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_ASSOC_AT_MS]));

-       printf("\n");
+       printf("\n\tcurrent time:\t%llu ms\n", now_ms);
         return NL_SKIP;
  }


Thanks,
Ben

> 
>   * @boottime_ns: CLOCK_BOOTTIME timestamp the frame was received at, this is
>   *      needed only for beacons and probe responses that update the scan cache.
> 
> 
>> + * @NL80211_STA_INFO_ASSOC_AT_MS: Timestamp of last association
> 
> _ASSOC_TIMESTAMP or something would make more sense too as the attribute
> name, and clearly the documentation needs to spell out the semantics
> here too.
> 
> johannes
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info
  2019-06-14 14:46   ` Ben Greear
@ 2019-06-14 14:56     ` Johannes Berg
  2019-06-14 15:14       ` Ben Greear
  0 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2019-06-14 14:56 UTC (permalink / raw)
  To: Ben Greear, linux-wireless

On Fri, 2019-06-14 at 07:46 -0700, Ben Greear wrote:
> 
> The point of my patch was to allow 'iw' to return a more precise time
> that the station has been associated, so I am not sure that BOOTIME is
> a good thing to use for that?

Depends what you want, really.

> +       if (sinfo[NL80211_STA_INFO_ASSOC_AT_MS])
> +               printf("\n\tassociated at:\t%llu ms",
> +                        (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_ASSOC_AT_MS]));
> 
> -       printf("\n");
> +       printf("\n\tcurrent time:\t%llu ms\n", now_ms);

Since you just print the time in milliseconds, and the current time in
milliseconds, you don't even really have any value in the wall clock.
Quite the opposite - this lends itself to subtracting to try to figure
out how long it was associated, which is the completely wrong thing to
do with wall clock - timezone adjustment could've happened inbetween,
for example!

I really see no point in trying to give the wall clock at assoc time. If
no timezone adjustment happened, you can just as well give the boottime
and have the userspace figure out the wall clock. If timezone adjustment
happened, then any calculations are wrong anyway, what would the point
be?

johannes


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info
  2019-06-14 14:56     ` Johannes Berg
@ 2019-06-14 15:14       ` Ben Greear
  2019-06-14 18:46         ` Johannes Berg
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Greear @ 2019-06-14 15:14 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless

On 6/14/19 7:56 AM, Johannes Berg wrote:
> On Fri, 2019-06-14 at 07:46 -0700, Ben Greear wrote:
>>
>> The point of my patch was to allow 'iw' to return a more precise time
>> that the station has been associated, so I am not sure that BOOTIME is
>> a good thing to use for that?
> 
> Depends what you want, really.
> 
>> +       if (sinfo[NL80211_STA_INFO_ASSOC_AT_MS])
>> +               printf("\n\tassociated at:\t%llu ms",
>> +                        (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_ASSOC_AT_MS]));
>>
>> -       printf("\n");
>> +       printf("\n\tcurrent time:\t%llu ms\n", now_ms);
> 
> Since you just print the time in milliseconds, and the current time in
> milliseconds, you don't even really have any value in the wall clock.
> Quite the opposite - this lends itself to subtracting to try to figure
> out how long it was associated, which is the completely wrong thing to
> do with wall clock - timezone adjustment could've happened inbetween,
> for example!
> 
> I really see no point in trying to give the wall clock at assoc time. If
> no timezone adjustment happened, you can just as well give the boottime
> and have the userspace figure out the wall clock. If timezone adjustment
> happened, then any calculations are wrong anyway, what would the point
> be?

So, maybe I return instead the elapsed time in the netlink API instead of a
timestamp.  I think that will give me the value that I am looking for,
and I can still print out the 'real' time in iw so any tools reading that
output and do some simple math and figure out the 'real' associated-at time.

If that sounds good to you, I'll code that up.

Thanks,
Ben

> 
> johannes
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info
  2019-06-14 15:14       ` Ben Greear
@ 2019-06-14 18:46         ` Johannes Berg
  0 siblings, 0 replies; 8+ messages in thread
From: Johannes Berg @ 2019-06-14 18:46 UTC (permalink / raw)
  To: Ben Greear, linux-wireless

On Fri, 2019-06-14 at 08:14 -0700, Ben Greear wrote:
> 
> 
> So, maybe I return instead the elapsed time in the netlink API instead of a
> timestamp.  I think that will give me the value that I am looking for,
> and I can still print out the 'real' time in iw so any tools reading that
> output and do some simple math and figure out the 'real' associated-at time.

I don't think that's good. There's a delay between filling the message
and then processing it in userspace. We had this in the scan code and
learned that was full of races.

The better thing is to use CLOCK_BOOTTIME nanoseconds, and then
userspace can use clock_gettime() to get the current time etc. and then
do whatever calculations it needs. If it wants to print the realtime
timestamp, it could even calculate that (with some jitter) by doing
clock_gettime() on both realtime and boottime clocks...

johannes


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15 17:21 [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info greearb
2019-04-15 17:21 ` [PATCH-v3 2/2] mac80211: add assoc-at-ms support greearb
2019-06-12 22:53   ` Ben Greear
2019-06-14 13:38 ` [PATCH-v3 1/2] wireless: Support assoc-at-ms timer in sta-info Johannes Berg
2019-06-14 14:46   ` Ben Greear
2019-06-14 14:56     ` Johannes Berg
2019-06-14 15:14       ` Ben Greear
2019-06-14 18:46         ` Johannes Berg

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox