[RFA] timestamp field

[RFA] timestamp field

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

Hi all,

This is a request for adoption of the timestamp field as documented on 
http://www.radiotap.org/suggested-fields/timestamp

I've attached the necessary wireshark patch, as well as a patch to the
Linux hardware simulator (mac80211_hwsim) to generate a dummy field for
experimentation.

I'd previously sent an RFC, but I didn't receive that much feedback so
I've just tried to be fairly generic here.

johannes

[-- Attachment #2: wireshark-radiotap-timstamp.patch --]
[-- Type: text/x-patch, Size: 9542 bytes --]

diff --git a/epan/dissectors/packet-ieee80211-radiotap-defs.h b/epan/dissectors/packet-ieee80211-radiotap-defs.h
index 25dd231bd7b9..6ea0ff23af14 100644
--- a/epan/dissectors/packet-ieee80211-radiotap-defs.h
+++ b/epan/dissectors/packet-ieee80211-radiotap-defs.h
@@ -207,6 +207,7 @@ enum ieee80211_radiotap_type {
 	IEEE80211_RADIOTAP_MCS = 19,
 	IEEE80211_RADIOTAP_AMPDU_STATUS = 20,
 	IEEE80211_RADIOTAP_VHT = 21,
+	IEEE80211_RADIOTAP_TIMESTAMP = 22,
 
 	/* valid in every it_present bitmap, even vendor namespaces */
 	IEEE80211_RADIOTAP_RADIOTAP_NAMESPACE = 29,
@@ -215,8 +216,8 @@ enum ieee80211_radiotap_type {
 };
 
 /* not (yet) defined Radiotap present flag */
-/* Bit 22 to 28 are not defined (in binary : 0001 1111 1100 0000 0000 0000 0000 0000 */
-#define IEEE80211_RADIOTAP_NOTDEFINED 0x1FC00000
+/* Bit 23 to 28 are not defined (in binary : 0001 1111 1000 0000 0000 0000 0000 0000 */
+#define IEEE80211_RADIOTAP_NOTDEFINED 0x1F800000
 
 /* Channel flags. */
 /* 0x00000001 through 0x00000008 undefined (reserved?) */
@@ -362,5 +363,19 @@ enum ieee80211_radiotap_type {
 #define IEEE80211_RADIOTAP_VHT_BW_20UUL		24
 #define IEEE80211_RADIOTAP_VHT_BW_20UUU		25
 
+/* for IEEE80211_RADIOTAP_TIMESTAMP */
+#define IEEE80211_RADIOTAP_TS_UNIT_MASK		0x0F
+#define IEEE80211_RADIOTAP_TS_UNIT_MSEC		0x00
+#define IEEE80211_RADIOTAP_TS_UNIT_USEC		0x01
+#define IEEE80211_RADIOTAP_TS_UNIT_NSEC		0x02
+#define IEEE80211_RADIOTAP_TS_SPOS_MASK		0xF0
+#define IEEE80211_RADIOTAP_TS_SPOS_SHIFT	4
+#define IEEE80211_RADIOTAP_TS_SPOS_MPDU		0x0
+#define IEEE80211_RADIOTAP_TS_SPOS_ACQ		0x1
+#define IEEE80211_RADIOTAP_TS_SPOS_EOF		0x2
+#define IEEE80211_RADIOTAP_TS_SPOS_UNDEF	0xF
+
+#define IEEE80211_RADIOTAP_TS_FLG_32BIT		0x01
+#define IEEE80211_RADIOTAP_TS_FLG_ACCURACY	0x02
 
 #endif				/* IEEE80211_RADIOTAP_H */
diff --git a/epan/dissectors/packet-ieee80211-radiotap-iter.c b/epan/dissectors/packet-ieee80211-radiotap-iter.c
index 799a78ecdc2d..1d1053d3dd8d 100644
--- a/epan/dissectors/packet-ieee80211-radiotap-iter.c
+++ b/epan/dissectors/packet-ieee80211-radiotap-iter.c
@@ -62,7 +62,8 @@ static const struct radiotap_align_size rtap_namespace_sizes[] = {
 	/* [IEEE80211_RADIOTAP_XCHANNEL] = 18 */		{ 0, 0 }, /* Unofficial, used by FreeBSD */
 	/* [IEEE80211_RADIOTAP_MCS] = 19 */			{ 1, 3 },
 	/* [IEEE80211_RADIOTAP_AMPDU_STATUS] = 20 */		{ 4, 8 },
-	/* [IEEE80211_RADIOTAP_VHT] = 21 */			{ 2, 12 }
+	/* [IEEE80211_RADIOTAP_VHT] = 21 */			{ 2, 12 },
+	/* [IEEE80211_RADIOTAP_TIMESTAMP] = 22 */		{ 8, 12 }
 
 	/*
 	 * add more here as they are defined in
diff --git a/epan/dissectors/packet-ieee80211-radiotap.c b/epan/dissectors/packet-ieee80211-radiotap.c
index 205b26427af9..728a9d5717cb 100644
--- a/epan/dissectors/packet-ieee80211-radiotap.c
+++ b/epan/dissectors/packet-ieee80211-radiotap.c
@@ -154,6 +154,14 @@ static int hf_radiotap_vht_datarate[4] = { -1, -1, -1, -1 };
 static int hf_radiotap_vht_gid = -1;
 static int hf_radiotap_vht_p_aid = -1;
 static int hf_radiotap_vht_user = -1;
+static int hf_radiotap_timestamp = -1;
+static int hf_radiotap_timestamp_ts = -1;
+static int hf_radiotap_timestamp_accuracy = -1;
+static int hf_radiotap_timestamp_unit = -1;
+static int hf_radiotap_timestamp_spos = -1;
+static int hf_radiotap_timestamp_flags = -1;
+static int hf_radiotap_timestamp_flags_32bit = -1;
+static int hf_radiotap_timestamp_flags_accuracy = -1;
 
 /* "Present" flags */
 static int hf_radiotap_present_tsft = -1;
@@ -176,6 +184,7 @@ static int hf_radiotap_present_xchannel = -1;
 static int hf_radiotap_present_mcs = -1;
 static int hf_radiotap_present_ampdu = -1;
 static int hf_radiotap_present_vht = -1;
+static int hf_radiotap_present_timestamp = -1;
 static int hf_radiotap_present_reserved = -1;
 static int hf_radiotap_present_rtap_ns = -1;
 static int hf_radiotap_present_vendor_ns = -1;
@@ -210,6 +219,8 @@ static gint ett_radiotap_ampdu_flags = -1;
 static gint ett_radiotap_vht = -1;
 static gint ett_radiotap_vht_known = -1;
 static gint ett_radiotap_vht_user = -1;
+static gint ett_radiotap_timestamp = -1;
+static gint ett_radiotap_timestamp_flags = -1;
 
 static expert_field ei_radiotap_data_past_header = EI_INIT;
 static expert_field ei_radiotap_present_reserved = EI_INIT;
@@ -430,6 +441,23 @@ static const true_false_string preamble_type = {
 	"Long",
 };
 
+static const value_string timestamp_unit[] = {
+	{ IEEE80211_RADIOTAP_TS_UNIT_MSEC, "msec" },
+	{ IEEE80211_RADIOTAP_TS_UNIT_USEC, "usec" },
+	{ IEEE80211_RADIOTAP_TS_UNIT_NSEC, "nsec" },
+	{ 0, NULL }
+};
+static value_string_ext timestamp_unit_ext = VALUE_STRING_EXT_INIT(timestamp_unit);
+
+static const value_string timestamp_spos[] = {
+	{ IEEE80211_RADIOTAP_TS_SPOS_MPDU, "first MPDU bit/symbol" },
+	{ IEEE80211_RADIOTAP_TS_SPOS_ACQ, "signal acquisition" },
+	{ IEEE80211_RADIOTAP_TS_SPOS_EOF, "end of frame" },
+	{ IEEE80211_RADIOTAP_TS_SPOS_UNDEF, "undefined" },
+	{ 0, NULL }
+};
+static value_string_ext timestamp_spos_ext = VALUE_STRING_EXT_INIT(timestamp_spos);
+
 /*
  * The NetBSD ieee80211_radiotap man page
  * (http://netbsd.gw.com/cgi-bin/man-cgi?ieee80211_radiotap+9+NetBSD-current)
@@ -744,6 +772,9 @@ dissect_radiotap(tvbuff_t * tvb, packet_info * pinfo, proto_tree * tree, void* u
 			proto_tree_add_item(present_tree,
 					    hf_radiotap_present_vht, tvb,
 					    offset + 4, 4, ENC_LITTLE_ENDIAN);
+			proto_tree_add_item(present_tree,
+					    hf_radiotap_present_timestamp, tvb,
+					    offset + 4, 4, ENC_LITTLE_ENDIAN);
 			ti = proto_tree_add_item(present_tree,
 					    hf_radiotap_present_reserved, tvb,
 					    offset + 4, 4, ENC_LITTLE_ENDIAN);
@@ -1739,6 +1770,32 @@ dissect_radiotap(tvbuff_t * tvb, packet_info * pinfo, proto_tree * tree, void* u
 
 			break;
 		}
+		case IEEE80211_RADIOTAP_TIMESTAMP:
+			if (tree) {
+				proto_item *it_root;
+				proto_tree *ts_tree, *flg_tree;
+
+				it_root = proto_tree_add_item(radiotap_tree, hf_radiotap_timestamp,
+						tvb, offset, 12, ENC_NA);
+				ts_tree = proto_item_add_subtree(it_root, ett_radiotap_timestamp);
+
+				proto_tree_add_item(ts_tree, hf_radiotap_timestamp_ts,
+						    tvb, offset, 8, ENC_LITTLE_ENDIAN);
+				if (tvb_get_letohs(tvb, offset + 11) & IEEE80211_RADIOTAP_TS_FLG_ACCURACY)
+					proto_tree_add_item(ts_tree, hf_radiotap_timestamp_accuracy,
+							    tvb, offset + 8, 2, ENC_LITTLE_ENDIAN);
+				proto_tree_add_item(ts_tree, hf_radiotap_timestamp_unit,
+						    tvb, offset + 10, 1, ENC_LITTLE_ENDIAN);
+				proto_tree_add_item(ts_tree, hf_radiotap_timestamp_spos,
+						    tvb, offset + 10, 1, ENC_LITTLE_ENDIAN);
+				flg_tree = proto_item_add_subtree(ts_tree, ett_radiotap_timestamp_flags);
+				proto_tree_add_item(flg_tree,
+						    hf_radiotap_timestamp_flags_32bit, tvb,
+						    offset + 11, 1, ENC_LITTLE_ENDIAN);
+				proto_tree_add_item(flg_tree,
+						    hf_radiotap_timestamp_flags_accuracy, tvb,
+						    offset + 11, 1, ENC_LITTLE_ENDIAN);
+			}
 		}
 	}
 
@@ -1927,6 +1984,11 @@ void proto_register_radiotap(void)
 		  FT_BOOLEAN, 32, TFS(&tfs_present_absent), RADIOTAP_MASK(VHT),
 		  "Specifies if the VHT field is present", HFILL}},
 
+		{&hf_radiotap_present_timestamp,
+		 {"frame timestamp", "radiotap.present.timestamp",
+		  FT_BOOLEAN, 32, TFS(&tfs_present_absent), RADIOTAP_MASK(TIMESTAMP),
+		  "Specifies if the timestamp field is present", HFILL}},
+
 		{&hf_radiotap_present_reserved,
 		 {"Reserved", "radiotap.present.reserved",
 		  FT_UINT32, BASE_HEX, NULL, IEEE80211_RADIOTAP_NOTDEFINED,
@@ -2586,6 +2648,48 @@ void proto_register_radiotap(void)
 		  FT_UINT16, BASE_DEC, NULL, 0x0,
 		  NULL, HFILL}},
 
+		{&hf_radiotap_timestamp,
+		 {"timestamp information", "radiotap.timestamp",
+		  FT_NONE, BASE_NONE, NULL, 0x0,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_ts,
+		 {"timestamp", "radiotap.timestamp.ts",
+		  FT_UINT64, BASE_DEC, NULL, 0x0,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_accuracy,
+		 {"accuracy", "radiotap.timestamp.accuracy",
+		  FT_UINT16, BASE_DEC, NULL, 0x0,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_unit,
+		 {"unit", "radiotap.timestamp.unit",
+		  FT_UINT8, BASE_DEC | BASE_EXT_STRING, &timestamp_unit_ext,
+		  IEEE80211_RADIOTAP_TS_UNIT_MASK,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_spos,
+		 {"sampling position", "radiotap.timestamp.samplingpos",
+		  FT_UINT8, BASE_DEC | BASE_EXT_STRING, &timestamp_spos_ext,
+		  IEEE80211_RADIOTAP_TS_SPOS_MASK,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_flags,
+		 {"timestamp flags", "radiotap.timestamp.flags",
+		  FT_NONE, BASE_NONE, NULL, 0x0,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_flags_32bit,
+		 {"32-bit counter", "radiotap.timestamp.flags.32bit",
+		  FT_BOOLEAN, 8, TFS(&tfs_yes_no), IEEE80211_RADIOTAP_TS_FLG_32BIT,
+		  NULL, HFILL}},
+
+		{&hf_radiotap_timestamp_flags_accuracy,
+		 {"accuracy field", "radiotap.timestamp.flags.accuracy",
+		  FT_BOOLEAN, 8, TFS(&tfs_present_absent), IEEE80211_RADIOTAP_TS_FLG_ACCURACY,
+		  NULL, HFILL}},
+
 		{&hf_radiotap_vendor_ns,
 		 {"Vendor namespace", "radiotap.vendor_namespace",
 		  FT_BYTES, BASE_NONE, NULL, 0x0,
@@ -2632,7 +2736,9 @@ void proto_register_radiotap(void)
 		&ett_radiotap_ampdu_flags,
 		&ett_radiotap_vht,
 		&ett_radiotap_vht_known,
-		&ett_radiotap_vht_user
+		&ett_radiotap_vht_user,
+		&ett_radiotap_timestamp,
+		&ett_radiotap_timestamp_flags,
 	};
 	static ei_register_info ei[] = {
 		{ &ei_radiotap_present, { "radiotap.present.radiotap_and_vendor", PI_MALFORMED, PI_ERROR, "Both radiotap and vendor namespace specified in bitmask word", EXPFILL }},

[-- Attachment #3: hwsim-radiotap-timestamp.patch --]
[-- Type: text/x-patch, Size: 1187 bytes --]

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index c32889a1e39c..6b1082797861 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -551,6 +551,13 @@ struct hwsim_radiotap_hdr {
 	u8 rt_rate;
 	__le16 rt_channel;
 	__le16 rt_chbitmask;
+	__le16 reserved;
+	struct {
+		__le64 ts;
+		__le16 accuracy;
+		u8 unit_pos;
+		u8 flags;
+	} ts;
 } __packed;
 
 struct hwsim_radiotap_ack_hdr {
@@ -831,6 +838,7 @@ static void mac80211_hwsim_monitor_rx(struct ieee80211_hw *hw,
 	hdr->hdr.it_present = cpu_to_le32((1 << IEEE80211_RADIOTAP_FLAGS) |
 					  (1 << IEEE80211_RADIOTAP_RATE) |
 					  (1 << IEEE80211_RADIOTAP_TSFT) |
+					  BIT(22) |
 					  (1 << IEEE80211_RADIOTAP_CHANNEL));
 	hdr->rt_tsft = __mac80211_hwsim_get_tsf(data);
 	hdr->rt_flags = 0;
@@ -842,6 +850,10 @@ static void mac80211_hwsim_monitor_rx(struct ieee80211_hw *hw,
 	else
 		flags |= IEEE80211_CHAN_CCK;
 	hdr->rt_chbitmask = cpu_to_le16(flags);
+	hdr->ts.ts = cpu_to_le64(ktime_get_boot_ns());
+	hdr->ts.accuracy = 0;
+	hdr->ts.unit_pos = 0x22;
+	hdr->ts.flags = 0x02;
 
 	skb->dev = hwsim_mon;
 	skb_set_mac_header(skb, 0);

Re: [RFA] timestamp field

[Johannes Berg]

Repost, since a few weeks have passed.

Any comments?

Simon, would this help you at all?

johannes

On Fri, 2015-12-04 at 14:41 +0100, Johannes Berg wrote:
> Hi all,
> 
> This is a request for adoption of the timestamp field as documented on 
> http://www.radiotap.org/suggested-fields/timestamp
> 
> I've attached the necessary wireshark patch, as well as a patch to the
> Linux hardware simulator (mac80211_hwsim) to generate a dummy field for
> experimentation.
> 
> I'd previously sent an RFC, but I didn't receive that much feedback so
> I've just tried to be fairly generic here.
> 
> johannes

Re: [RFA] timestamp field

[Simon Barber]

I like this proposal. One aspect that is missing is to define the 
meaning of the timestamp with aggregates. Should this field be present 
for all subframes, or just the first/last? Should the timestamp refer to 
the subframes, or the whole A-MPDU? Currently what I see in different 
generators is the MACTIME field is either the same for all subframes, 
and always refers to the whole A-MPDU - not individual subframes. For 
one generator the MACTIME was 0 for subframes after the 1st. Sometimes 
the signal strength is only present on the last subframe.

Macbooks generate radtiotap files with incorrect rate fields for 
subframes with CRC errors.

If you look in the wireshark gerrit, you will see I have been porting my 
timeline viewer code over to the current devel mainline.

Simon

On 1/20/2016 4:42 AM, Johannes Berg wrote:
> Repost, since a few weeks have passed.
>
> Any comments?
>
> Simon, would this help you at all?
>
> johannes
>
> On Fri, 2015-12-04 at 14:41 +0100, Johannes Berg wrote:
>> Hi all,
>>
>> This is a request for adoption of the timestamp field as documented on
>> http://www.radiotap.org/suggested-fields/timestamp
>>
>> I've attached the necessary wireshark patch, as well as a patch to the
>> Linux hardware simulator (mac80211_hwsim) to generate a dummy field for
>> experimentation.
>>
>> I'd previously sent an RFC, but I didn't receive that much feedback so
>> I've just tried to be fairly generic here.
>>
>> johannes

Re: [RFA] timestamp field

[Johannes Berg]

On Wed, 2016-01-20 at 09:46 -0800, Simon Barber wrote:
> I like this proposal. One aspect that is missing is to define the 
> meaning of the timestamp with aggregates. Should this field be
> present for all subframes, or just the first/last? Should the
> timestamp refer to the subframes, or the whole A-MPDU? Currently what
> I see in different generators is the MACTIME field is either the same
> for all subframes, and always refers to the whole A-MPDU - not
> individual subframes. For one generator the MACTIME was 0 for
> subframes after the 1st. Sometimes the signal strength is only
> present on the last subframe.

Yeah, I had actually noticed this - our device also has the PHY
acquisition timestamp of the A-MPDU, in all the frames. We should
probably elide it in all but the first, but if we somehow lost the
first frame that would be bad. I'm not even sure you can identify that
today at all though.

I don't think we can *standardise* the behaviour, but perhaps providing
flags for the different behaviours would at least help you identify it?

Any thoughts what flags would be useful?

> Macbooks generate radtiotap files with incorrect rate fields for 
> subframes with CRC errors.

That's a bit odd. Talk to Broadcom? ;-)

> If you look in the wireshark gerrit, you will see I have been porting
> my timeline viewer code over to the current devel mainline.
> 

Very nice! :)

johannes

Re: [RFA] timestamp field

[Simon Barber]

My wireshark timeline code is out for early review.

https://code.wireshark.org/review/#/c/13694

Simon

Sent with AquaMail for Android
http://www.aqua-mail.com


On January 20, 2016 11:40:14 PM Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> 
wrote:

> On Wed, 2016-01-20 at 09:46 -0800, Simon Barber wrote:
>> I like this proposal. One aspect that is missing is to define the
>> meaning of the timestamp with aggregates. Should this field be
>> present for all subframes, or just the first/last? Should the
>> timestamp refer to the subframes, or the whole A-MPDU? Currently what
>> I see in different generators is the MACTIME field is either the same
>> for all subframes, and always refers to the whole A-MPDU - not
>> individual subframes. For one generator the MACTIME was 0 for
>> subframes after the 1st. Sometimes the signal strength is only
>> present on the last subframe.
>
> Yeah, I had actually noticed this - our device also has the PHY
> acquisition timestamp of the A-MPDU, in all the frames. We should
> probably elide it in all but the first, but if we somehow lost the
> first frame that would be bad. I'm not even sure you can identify that
> today at all though.
>
> I don't think we can *standardise* the behaviour, but perhaps providing
> flags for the different behaviours would at least help you identify it?
>
> Any thoughts what flags would be useful?
>
>> Macbooks generate radtiotap files with incorrect rate fields for
>> subframes with CRC errors.
>
> That's a bit odd. Talk to Broadcom? ;-)
>
>> If you look in the wireshark gerrit, you will see I have been porting
>> my timeline viewer code over to the current devel mainline.
>>
>
> Very nice! :)
>
> johannes

Re: [RFA] timestamp field

[Johannes Berg]

On Wed, 2016-01-20 at 09:46 -0800, Simon Barber wrote:
> I like this proposal. One aspect that is missing is to define the 
> meaning of the timestamp with aggregates. 

Actually, I think the field kinda already covers that.

If you have sampling position 0, then it ought to be of *that* MPDU, so
probably not present in any subframes but the first, unless you can
sample a different timestamp for each.

If you have sampling position 1 (PLCP start) then obviously that's at
the A-MPDU level and it doesn't matter if it's present in all or the
first only, in some ways. (Note that we found that Broadcom generates
an invalid A-MPDU reference field...)

Sampling position 2 - end of frame - I'll clarify to be "end of PPDU",
I think that's what ath* devices create.

johannes

Re: [RFA] timestamp field

[Johannes Berg]

On Fri, 2015-12-04 at 14:41 +0100, Johannes Berg wrote:
> Hi all,
> 
> This is a request for adoption of the timestamp field as documented
> on http://www.radiotap.org/suggested-fields/timestamp
> 
Repost, since I made a small change:

 * changed sampling position 2 to be "end of PPDU"
 * added sampling position 3 to mean
   "end of MPDU (after FCS)"

johannes