Allow drivers to indicate their mactime is at RX completion and adjust
for this in mac80211. Based on similar code by Johannes Berg.
Signed-off-by: Thomas Pedersen <[email protected]>
---
include/net/mac80211.h | 4 ++++
net/mac80211/ibss.c | 29 +++++--------------------
net/mac80211/ieee80211_i.h | 9 ++++++++
net/mac80211/mesh_sync.c | 43 ++++++-------------------------------
net/mac80211/rx.c | 14 ++++++++++---
net/mac80211/util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 85 insertions(+), 64 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 00b7204..a2b9b52 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -704,6 +704,9 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
* field) is valid and contains the time the first symbol of the MPDU
* was received. This is useful in monitor mode and for proper IBSS
* merging.
+ * @RX_FLAG_MACTIME_END: The timestamp passed in the RX status (@mactime
+ * field) is valid and contains the time the last symbol of the MPDU
+ * was received.
* @RX_FLAG_SHORTPRE: Short preamble was used for this frame
* @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index
* @RX_FLAG_40MHZ: HT40 (40 MHz) was used
@@ -748,6 +751,7 @@ enum mac80211_rx_flags {
RX_FLAG_AMPDU_IS_LAST = BIT(18),
RX_FLAG_AMPDU_DELIM_CRC_ERROR = BIT(19),
RX_FLAG_AMPDU_DELIM_CRC_KNOWN = BIT(20),
+ RX_FLAG_MACTIME_END = BIT(21),
};
/**
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index c7386b2..92500d8 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -543,30 +543,11 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
if (ether_addr_equal(cbss->bssid, sdata->u.ibss.bssid))
goto put_bss;
- if (rx_status->flag & RX_FLAG_MACTIME_MPDU) {
- /*
- * For correct IBSS merging we need mactime; since mactime is
- * defined as the time the first data symbol of the frame hits
- * the PHY, and the timestamp of the beacon is defined as "the
- * time that the data symbol containing the first bit of the
- * timestamp is transmitted to the PHY plus the transmitting
- * STA's delays through its local PHY from the MAC-PHY
- * interface to its interface with the WM" (802.11 11.1.2)
- * - equals the time this bit arrives at the receiver - we have
- * to take into account the offset between the two.
- *
- * E.g. at 1 MBit that means mactime is 192 usec earlier
- * (=24 bytes * 8 usecs/byte) than the beacon timestamp.
- */
- int rate;
-
- if (rx_status->flag & RX_FLAG_HT)
- rate = 65; /* TODO: HT rates */
- else
- rate = local->hw.wiphy->bands[band]->
- bitrates[rx_status->rate_idx].bitrate;
-
- rx_timestamp = rx_status->mactime + (24 * 8 * 10 / rate);
+ if (ieee80211_have_rx_timestamp(rx_status)) {
+ /* time when timestamp field was received */
+ rx_timestamp =
+ ieee80211_calculate_rx_timestamp(local, rx_status,
+ len + 4, 24);
} else {
/*
* second best option: get current TSF
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3026519..6162342 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1251,7 +1251,16 @@ static inline int ieee80211_bssid_match(const u8 *raddr, const u8 *addr)
is_broadcast_ether_addr(raddr);
}
+static inline bool
+ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
+{
+ return status->flag & (RX_FLAG_MACTIME_MPDU | RX_FLAG_MACTIME_END);
+}
+u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
+ struct ieee80211_rx_status *status,
+ unsigned int mpdu_len,
+ unsigned int mpdu_offset);
int ieee80211_hw_config(struct ieee80211_local *local, u32 changed);
void ieee80211_tx_set_protected(struct ieee80211_tx_data *tx);
void ieee80211_bss_info_change_notify(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/mesh_sync.c b/net/mac80211/mesh_sync.c
index 407c870..99e792b 100644
--- a/net/mac80211/mesh_sync.c
+++ b/net/mac80211/mesh_sync.c
@@ -116,43 +116,12 @@ static void mesh_sync_offset_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
goto no_sync;
}
- if (rx_status->flag & RX_FLAG_MACTIME_MPDU && rx_status->mactime) {
- /*
- * The mactime is defined as the time the first data symbol
- * of the frame hits the PHY, and the timestamp of the beacon
- * is defined as "the time that the data symbol containing the
- * first bit of the timestamp is transmitted to the PHY plus
- * the transmitting STA's delays through its local PHY from the
- * MAC-PHY interface to its interface with the WM" (802.11
- * 11.1.2)
- *
- * T_r, in 13.13.2.2.2, is just defined as "the frame reception
- * time" but we unless we interpret that time to be the same
- * time of the beacon timestamp, the offset calculation will be
- * off. Below we adjust t_r to be "the time at which the first
- * symbol of the timestamp element in the beacon is received".
- * This correction depends on the rate.
- *
- * Based on similar code in ibss.c
- */
- int rate;
-
- if (rx_status->flag & RX_FLAG_HT) {
- /* TODO:
- * In principle there could be HT-beacons (Dual Beacon
- * HT Operation options), but for now ignore them and
- * just use the primary (i.e. non-HT) beacons for
- * synchronization.
- * */
- goto no_sync;
- } else
- rate = local->hw.wiphy->bands[rx_status->band]->
- bitrates[rx_status->rate_idx].bitrate;
-
- /* 24 bytes of header * 8 bits/byte *
- * 10*(100 Kbps)/Mbps / rate (100 Kbps)*/
- t_r = rx_status->mactime + (24 * 8 * 10 / rate);
- }
+ if (ieee80211_have_rx_timestamp(rx_status))
+ /* time when timestamp field was received */
+ t_r = ieee80211_calculate_rx_timestamp(local, rx_status,
+ 24 + 12 +
+ elems->total_len + 4,
+ 24);
/* Timing offset calculation (see 13.13.2.2.2) */
t_t = le64_to_cpu(mgmt->u.beacon.timestamp);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 8c1f152..9898fda 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -82,7 +82,7 @@ ieee80211_rx_radiotap_len(struct ieee80211_local *local,
/* always present fields */
len = sizeof(struct ieee80211_radiotap_header) + 9;
- if (status->flag & RX_FLAG_MACTIME_MPDU)
+ if (ieee80211_have_rx_timestamp(status))
len += 8;
if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
len += 1;
@@ -118,6 +118,11 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
struct ieee80211_radiotap_header *rthdr;
unsigned char *pos;
u16 rx_flags = 0;
+ int mpdulen;
+
+ mpdulen = skb->len;
+ if (!(has_fcs && (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)))
+ mpdulen += 4;
rthdr = (struct ieee80211_radiotap_header *)skb_push(skb, rtap_len);
memset(rthdr, 0, rtap_len);
@@ -135,8 +140,11 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
/* the order of the following fields is important */
/* IEEE80211_RADIOTAP_TSFT */
- if (status->flag & RX_FLAG_MACTIME_MPDU) {
- put_unaligned_le64(status->mactime, pos);
+ if (ieee80211_have_rx_timestamp(status)) {
+ put_unaligned_le64(
+ ieee80211_calculate_rx_timestamp(local, status,
+ mpdulen, 0),
+ pos);
rthdr->it_present |=
cpu_to_le32(1 << IEEE80211_RADIOTAP_TSFT);
pos += 8;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 9556391..9d7271e 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1990,3 +1990,53 @@ u8 ieee80211_mcs_to_chains(const struct ieee80211_mcs_info *mcs)
return 2;
return 1;
}
+
+/**
+ * ieee80211_calculate_rx_timestamp - calculate timestamp in frame
+ * @status: RX status
+ * @mpdu_len: total MPDU length (including FCS)
+ * @mpdu_offset: offset into MPDU to calculate timestamp at
+ *
+ * This function calculates the RX timestamp at the given MPDU offset, taking
+ * into account what the RX timestamp was. An offset of 0 will just normalize
+ * the timestamp to TSF at beginning of MPDU reception.
+ */
+u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
+ struct ieee80211_rx_status *status,
+ unsigned int mpdu_len,
+ unsigned int mpdu_offset)
+{
+ u64 ts = status->mactime;
+ struct rate_info ri;
+ u16 rate;
+
+ if (WARN_ON(!ieee80211_have_rx_timestamp(status)))
+ return 0;
+
+ memset(&ri, 0, sizeof(ri));
+
+ /* Fill cfg80211 rate info */
+ if (status->flag & RX_FLAG_HT) {
+ ri.mcs = status->rate_idx;
+ ri.flags |= RATE_INFO_FLAGS_MCS;
+ if (status->flag & RX_FLAG_40MHZ)
+ ri.flags |= RATE_INFO_FLAGS_40_MHZ_WIDTH;
+ if (status->flag & RX_FLAG_SHORT_GI)
+ ri.flags |= RATE_INFO_FLAGS_SHORT_GI;
+ } else {
+ struct ieee80211_supported_band *sband;
+
+ sband = local->hw.wiphy->bands[status->band];
+ ri.legacy = sband->bitrates[status->rate_idx].bitrate;
+ }
+
+ rate = cfg80211_calculate_bitrate(&ri);
+
+ /* rewind from end of MPDU */
+ if (status->flag & RX_FLAG_MACTIME_END)
+ ts -= mpdu_len * 8 * 10 / rate;
+
+ ts += mpdu_offset * 8 * 10 / rate;
+
+ return ts;
+}
--
1.7.10.4
On Mon, 2012-11-12 at 16:48 -0800, Adrian Chadd wrote:
> So for the sake of being complete(ish), the Atheros hardware looks
> like it behaves slightly different for single frames versus
> aggregates.
>
> So I guess it depends upon what kind of accuracy you're looking for?
> Would it be worth implementing it as a per-frame flag, so the driver
> has a chance of setting the flag for aggregates versus non-aggregates?
It already is a per frame flag :)
johannes
So for the sake of being complete(ish), the Atheros hardware looks
like it behaves slightly different for single frames versus
aggregates.
So I guess it depends upon what kind of accuracy you're looking for?
Would it be worth implementing it as a per-frame flag, so the driver
has a chance of setting the flag for aggregates versus non-aggregates?
Adrian
On Mon, 2012-11-12 at 10:38 -0800, Thomas Pedersen wrote:
> + if (ieee80211_have_rx_timestamp(rx_status))
> + /* time when timestamp field was received */
> + t_r = ieee80211_calculate_rx_timestamp(local, rx_status,
> + 24 + 12 +
> + elems->total_len + 4,
> + 24);
This doesn't seem quite right, the FCS isn't accounted for correctly?
I think it might be wortwhile to pass the SKB to the function instead of
rx_status though, then it could get skb->len and check whether or not
FCS was before the timestamp (which, btw, you should mention in the
documentation for the new flag!)
And then I suspect the check for IEEE80211_HW_RX_INCLUDES_FCS is also
incorrect -- even if the driver didn't report the FCS maybe the
timestamping semantic was such that it was after the FCS?
This seems to need a little fixing up, but other than that looks good.
johannes
On Mon, Nov 12, 2012 at 12:11 PM, Javier Cardona <[email protected]> wrote:
> Thomas,
>
> On Mon, Nov 12, 2012 at 10:38 AM, Thomas Pedersen <[email protected]> wrote:
>> Allow drivers to indicate their mactime is at RX completion and adjust
>> for this in mac80211. Based on similar code by Johannes Berg.
>>
>> Signed-off-by: Thomas Pedersen <[email protected]>
>> ---
>> include/net/mac80211.h | 4 ++++
>> net/mac80211/ibss.c | 29 +++++--------------------
>> net/mac80211/ieee80211_i.h | 9 ++++++++
>> net/mac80211/mesh_sync.c | 43 ++++++-------------------------------
>> net/mac80211/rx.c | 14 ++++++++++---
>> net/mac80211/util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 85 insertions(+), 64 deletions(-)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index 00b7204..a2b9b52 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -704,6 +704,9 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
>> * field) is valid and contains the time the first symbol of the MPDU
>> * was received. This is useful in monitor mode and for proper IBSS
>> * merging.
>> + * @RX_FLAG_MACTIME_END: The timestamp passed in the RX status (@mactime
>> + * field) is valid and contains the time the last symbol of the MPDU
>> + * was received.
>
> So with this change, you are suggesting that drivers set at most *one*
> of RX_FLAG_MACTIME_MPDU (if they report mactime at the start of the
> frame) or RX_FLAG_MACTIME_END (at the end)? If so, at the risk of
> being too verbose, I would suggest to use
> RX_FLAG_MACTIME_MPDU_{START,END} or to mention this in the comment:
> "This flag should not be set with RX_FLAG_MACTIME_MPDU"
OK, I'll rename RX_FLAG_MACTIME_MDPU to RX_FLAG_MACTIME START in the v2 then.
>> * @RX_FLAG_SHORTPRE: Short preamble was used for this frame
>> * @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index
>> * @RX_FLAG_40MHZ: HT40 (40 MHz) was used
>> @@ -748,6 +751,7 @@ enum mac80211_rx_flags {
>> RX_FLAG_AMPDU_IS_LAST = BIT(18),
>> RX_FLAG_AMPDU_DELIM_CRC_ERROR = BIT(19),
>> RX_FLAG_AMPDU_DELIM_CRC_KNOWN = BIT(20),
>> + RX_FLAG_MACTIME_END = BIT(21),
>> };
>>
>> /**
>> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
>> index c7386b2..92500d8 100644
>> --- a/net/mac80211/ibss.c
>> +++ b/net/mac80211/ibss.c
>> @@ -543,30 +543,11 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
>> if (ether_addr_equal(cbss->bssid, sdata->u.ibss.bssid))
>> goto put_bss;
>>
>> - if (rx_status->flag & RX_FLAG_MACTIME_MPDU) {
>> - /*
>> - * For correct IBSS merging we need mactime; since mactime is
>> - * defined as the time the first data symbol of the frame hits
>> - * the PHY, and the timestamp of the beacon is defined as "the
>> - * time that the data symbol containing the first bit of the
>> - * timestamp is transmitted to the PHY plus the transmitting
>> - * STA's delays through its local PHY from the MAC-PHY
>> - * interface to its interface with the WM" (802.11 11.1.2)
>> - * - equals the time this bit arrives at the receiver - we have
>> - * to take into account the offset between the two.
>> - *
>> - * E.g. at 1 MBit that means mactime is 192 usec earlier
>> - * (=24 bytes * 8 usecs/byte) than the beacon timestamp.
>> - */
>> - int rate;
>> -
>> - if (rx_status->flag & RX_FLAG_HT)
>> - rate = 65; /* TODO: HT rates */
>
> Ah, and I see that in your proposed
> ieee80211_calculate_rx_timestamp() you take care of HT rates... nice.
>
>> - else
>> - rate = local->hw.wiphy->bands[band]->
>> - bitrates[rx_status->rate_idx].bitrate;
>> -
>> - rx_timestamp = rx_status->mactime + (24 * 8 * 10 / rate);
>> + if (ieee80211_have_rx_timestamp(rx_status)) {
>> + /* time when timestamp field was received */
>> + rx_timestamp =
>> + ieee80211_calculate_rx_timestamp(local, rx_status,
>> + len + 4, 24);
>> } else {
>> /*
>> * second best option: get current TSF
>> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
>> index 3026519..6162342 100644
>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -1251,7 +1251,16 @@ static inline int ieee80211_bssid_match(const u8 *raddr, const u8 *addr)
>> is_broadcast_ether_addr(raddr);
>> }
>>
>> +static inline bool
>> +ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
>> +{
>
> Maybe...
>
> WARN_ON((status->flag & RX_FLAG_MACTIME_MPDU) &&
> (status->flag & RX_FLAG_MACTIME_END)) ?
>
OK.
Thomas
Thomas,
On Mon, Nov 12, 2012 at 10:38 AM, Thomas Pedersen <[email protected]> wrote:
> Allow drivers to indicate their mactime is at RX completion and adjust
> for this in mac80211. Based on similar code by Johannes Berg.
>
> Signed-off-by: Thomas Pedersen <[email protected]>
> ---
> include/net/mac80211.h | 4 ++++
> net/mac80211/ibss.c | 29 +++++--------------------
> net/mac80211/ieee80211_i.h | 9 ++++++++
> net/mac80211/mesh_sync.c | 43 ++++++-------------------------------
> net/mac80211/rx.c | 14 ++++++++++---
> net/mac80211/util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 85 insertions(+), 64 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 00b7204..a2b9b52 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -704,6 +704,9 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
> * field) is valid and contains the time the first symbol of the MPDU
> * was received. This is useful in monitor mode and for proper IBSS
> * merging.
> + * @RX_FLAG_MACTIME_END: The timestamp passed in the RX status (@mactime
> + * field) is valid and contains the time the last symbol of the MPDU
> + * was received.
So with this change, you are suggesting that drivers set at most *one*
of RX_FLAG_MACTIME_MPDU (if they report mactime at the start of the
frame) or RX_FLAG_MACTIME_END (at the end)? If so, at the risk of
being too verbose, I would suggest to use
RX_FLAG_MACTIME_MPDU_{START,END} or to mention this in the comment:
"This flag should not be set with RX_FLAG_MACTIME_MPDU"
> * @RX_FLAG_SHORTPRE: Short preamble was used for this frame
> * @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index
> * @RX_FLAG_40MHZ: HT40 (40 MHz) was used
> @@ -748,6 +751,7 @@ enum mac80211_rx_flags {
> RX_FLAG_AMPDU_IS_LAST = BIT(18),
> RX_FLAG_AMPDU_DELIM_CRC_ERROR = BIT(19),
> RX_FLAG_AMPDU_DELIM_CRC_KNOWN = BIT(20),
> + RX_FLAG_MACTIME_END = BIT(21),
> };
>
> /**
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index c7386b2..92500d8 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -543,30 +543,11 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> if (ether_addr_equal(cbss->bssid, sdata->u.ibss.bssid))
> goto put_bss;
>
> - if (rx_status->flag & RX_FLAG_MACTIME_MPDU) {
> - /*
> - * For correct IBSS merging we need mactime; since mactime is
> - * defined as the time the first data symbol of the frame hits
> - * the PHY, and the timestamp of the beacon is defined as "the
> - * time that the data symbol containing the first bit of the
> - * timestamp is transmitted to the PHY plus the transmitting
> - * STA's delays through its local PHY from the MAC-PHY
> - * interface to its interface with the WM" (802.11 11.1.2)
> - * - equals the time this bit arrives at the receiver - we have
> - * to take into account the offset between the two.
> - *
> - * E.g. at 1 MBit that means mactime is 192 usec earlier
> - * (=24 bytes * 8 usecs/byte) than the beacon timestamp.
> - */
> - int rate;
> -
> - if (rx_status->flag & RX_FLAG_HT)
> - rate = 65; /* TODO: HT rates */
Ah, and I see that in your proposed
ieee80211_calculate_rx_timestamp() you take care of HT rates... nice.
> - else
> - rate = local->hw.wiphy->bands[band]->
> - bitrates[rx_status->rate_idx].bitrate;
> -
> - rx_timestamp = rx_status->mactime + (24 * 8 * 10 / rate);
> + if (ieee80211_have_rx_timestamp(rx_status)) {
> + /* time when timestamp field was received */
> + rx_timestamp =
> + ieee80211_calculate_rx_timestamp(local, rx_status,
> + len + 4, 24);
> } else {
> /*
> * second best option: get current TSF
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 3026519..6162342 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1251,7 +1251,16 @@ static inline int ieee80211_bssid_match(const u8 *raddr, const u8 *addr)
> is_broadcast_ether_addr(raddr);
> }
>
> +static inline bool
> +ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
> +{
Maybe...
WARN_ON((status->flag & RX_FLAG_MACTIME_MPDU) &&
(status->flag & RX_FLAG_MACTIME_END)) ?
> + return status->flag & (RX_FLAG_MACTIME_MPDU | RX_FLAG_MACTIME_END);
> +}
>
> +u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
> + struct ieee80211_rx_status *status,
> + unsigned int mpdu_len,
> + unsigned int mpdu_offset);
> int ieee80211_hw_config(struct ieee80211_local *local, u32 changed);
> void ieee80211_tx_set_protected(struct ieee80211_tx_data *tx);
> void ieee80211_bss_info_change_notify(struct ieee80211_sub_if_data *sdata,
> diff --git a/net/mac80211/mesh_sync.c b/net/mac80211/mesh_sync.c
> index 407c870..99e792b 100644
> --- a/net/mac80211/mesh_sync.c
> +++ b/net/mac80211/mesh_sync.c
> @@ -116,43 +116,12 @@ static void mesh_sync_offset_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
> goto no_sync;
> }
>
> - if (rx_status->flag & RX_FLAG_MACTIME_MPDU && rx_status->mactime) {
> - /*
> - * The mactime is defined as the time the first data symbol
> - * of the frame hits the PHY, and the timestamp of the beacon
> - * is defined as "the time that the data symbol containing the
> - * first bit of the timestamp is transmitted to the PHY plus
> - * the transmitting STA's delays through its local PHY from the
> - * MAC-PHY interface to its interface with the WM" (802.11
> - * 11.1.2)
> - *
> - * T_r, in 13.13.2.2.2, is just defined as "the frame reception
> - * time" but we unless we interpret that time to be the same
> - * time of the beacon timestamp, the offset calculation will be
> - * off. Below we adjust t_r to be "the time at which the first
> - * symbol of the timestamp element in the beacon is received".
> - * This correction depends on the rate.
> - *
> - * Based on similar code in ibss.c
> - */
> - int rate;
> -
> - if (rx_status->flag & RX_FLAG_HT) {
> - /* TODO:
> - * In principle there could be HT-beacons (Dual Beacon
> - * HT Operation options), but for now ignore them and
> - * just use the primary (i.e. non-HT) beacons for
> - * synchronization.
> - * */
> - goto no_sync;
> - } else
> - rate = local->hw.wiphy->bands[rx_status->band]->
> - bitrates[rx_status->rate_idx].bitrate;
> -
> - /* 24 bytes of header * 8 bits/byte *
> - * 10*(100 Kbps)/Mbps / rate (100 Kbps)*/
> - t_r = rx_status->mactime + (24 * 8 * 10 / rate);
> - }
> + if (ieee80211_have_rx_timestamp(rx_status))
> + /* time when timestamp field was received */
> + t_r = ieee80211_calculate_rx_timestamp(local, rx_status,
> + 24 + 12 +
> + elems->total_len + 4,
> + 24);
>
> /* Timing offset calculation (see 13.13.2.2.2) */
> t_t = le64_to_cpu(mgmt->u.beacon.timestamp);
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 8c1f152..9898fda 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -82,7 +82,7 @@ ieee80211_rx_radiotap_len(struct ieee80211_local *local,
> /* always present fields */
> len = sizeof(struct ieee80211_radiotap_header) + 9;
>
> - if (status->flag & RX_FLAG_MACTIME_MPDU)
> + if (ieee80211_have_rx_timestamp(status))
> len += 8;
> if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
> len += 1;
> @@ -118,6 +118,11 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
> struct ieee80211_radiotap_header *rthdr;
> unsigned char *pos;
> u16 rx_flags = 0;
> + int mpdulen;
> +
> + mpdulen = skb->len;
> + if (!(has_fcs && (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)))
> + mpdulen += 4;
>
> rthdr = (struct ieee80211_radiotap_header *)skb_push(skb, rtap_len);
> memset(rthdr, 0, rtap_len);
> @@ -135,8 +140,11 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
> /* the order of the following fields is important */
>
> /* IEEE80211_RADIOTAP_TSFT */
> - if (status->flag & RX_FLAG_MACTIME_MPDU) {
> - put_unaligned_le64(status->mactime, pos);
> + if (ieee80211_have_rx_timestamp(status)) {
> + put_unaligned_le64(
> + ieee80211_calculate_rx_timestamp(local, status,
> + mpdulen, 0),
> + pos);
> rthdr->it_present |=
> cpu_to_le32(1 << IEEE80211_RADIOTAP_TSFT);
> pos += 8;
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 9556391..9d7271e 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -1990,3 +1990,53 @@ u8 ieee80211_mcs_to_chains(const struct ieee80211_mcs_info *mcs)
> return 2;
> return 1;
> }
> +
> +/**
> + * ieee80211_calculate_rx_timestamp - calculate timestamp in frame
> + * @status: RX status
> + * @mpdu_len: total MPDU length (including FCS)
> + * @mpdu_offset: offset into MPDU to calculate timestamp at
> + *
> + * This function calculates the RX timestamp at the given MPDU offset, taking
> + * into account what the RX timestamp was. An offset of 0 will just normalize
> + * the timestamp to TSF at beginning of MPDU reception.
> + */
> +u64 ieee80211_calculate_rx_timestamp(struct ieee80211_local *local,
> + struct ieee80211_rx_status *status,
> + unsigned int mpdu_len,
> + unsigned int mpdu_offset)
> +{
> + u64 ts = status->mactime;
> + struct rate_info ri;
> + u16 rate;
> +
> + if (WARN_ON(!ieee80211_have_rx_timestamp(status)))
> + return 0;
> +
> + memset(&ri, 0, sizeof(ri));
> +
> + /* Fill cfg80211 rate info */
> + if (status->flag & RX_FLAG_HT) {
> + ri.mcs = status->rate_idx;
> + ri.flags |= RATE_INFO_FLAGS_MCS;
> + if (status->flag & RX_FLAG_40MHZ)
> + ri.flags |= RATE_INFO_FLAGS_40_MHZ_WIDTH;
> + if (status->flag & RX_FLAG_SHORT_GI)
> + ri.flags |= RATE_INFO_FLAGS_SHORT_GI;
> + } else {
> + struct ieee80211_supported_band *sband;
> +
> + sband = local->hw.wiphy->bands[status->band];
> + ri.legacy = sband->bitrates[status->rate_idx].bitrate;
> + }
> +
> + rate = cfg80211_calculate_bitrate(&ri);
> +
> + /* rewind from end of MPDU */
> + if (status->flag & RX_FLAG_MACTIME_END)
> + ts -= mpdu_len * 8 * 10 / rate;
> +
> + ts += mpdu_offset * 8 * 10 / rate;
> +
> + return ts;
> +}
> --
> 1.7.10.4
>
> --
Thanks!
Javier
--
Javier Cardona
cozybit Inc.
http://www.cozybit.com
On Mon, Nov 12, 2012 at 12:42 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2012-11-12 at 10:38 -0800, Thomas Pedersen wrote:
>
>> + if (ieee80211_have_rx_timestamp(rx_status))
>> + /* time when timestamp field was received */
>> + t_r = ieee80211_calculate_rx_timestamp(local, rx_status,
>> + 24 + 12 +
>> + elems->total_len + 4,
>> + 24);
>
> This doesn't seem quite right, the FCS isn't accounted for correctly?
That's what the +4 is for. Is this wrong?
> I think it might be wortwhile to pass the SKB to the function instead of
> rx_status though, then it could get skb->len and check whether or not
> FCS was before the timestamp
How would you check this?
> (which, btw, you should mention in the documentation for the new flag!)
I'll make it clear MACTIME_END includes the FCS.
> And then I suspect the check for IEEE80211_HW_RX_INCLUDES_FCS is also
> incorrect -- even if the driver didn't report the FCS maybe the
> timestamping semantic was such that it was after the FCS?
Right, if the driver doesn't report the FCS, the radiotap code will
add an extra 4 bytes to account for this.
Thomas
On Mon, 2012-11-12 at 13:52 -0800, Thomas Pedersen wrote:
> On Mon, Nov 12, 2012 at 12:42 PM, Johannes Berg
> <[email protected]> wrote:
> > On Mon, 2012-11-12 at 10:38 -0800, Thomas Pedersen wrote:
> >
> >> + if (ieee80211_have_rx_timestamp(rx_status))
> >> + /* time when timestamp field was received */
> >> + t_r = ieee80211_calculate_rx_timestamp(local, rx_status,
> >> + 24 + 12 +
> >> + elems->total_len + 4,
> >> + 24);
> >
> > This doesn't seem quite right, the FCS isn't accounted for correctly?
>
> That's what the +4 is for. Is this wrong?
No, ok, I guess that's correct then. I didn't do the calculations to
check. Why not use skb->len though, rather than 24+12+...?
Btw, I think there's a define for FCS_LEN somewhere, that might be
worthwhile to use just for documentation.
> > I think it might be wortwhile to pass the SKB to the function instead of
> > rx_status though, then it could get skb->len and check whether or not
> > FCS was before the timestamp
>
> How would you check this?
Never mind, you can't know.
> > (which, btw, you should mention in the documentation for the new flag!)
>
> I'll make it clear MACTIME_END includes the FCS.
Ok.
> > And then I suspect the check for IEEE80211_HW_RX_INCLUDES_FCS is also
> > incorrect -- even if the driver didn't report the FCS maybe the
> > timestamping semantic was such that it was after the FCS?
>
> Right, if the driver doesn't report the FCS, the radiotap code will
> add an extra 4 bytes to account for this.
Oh, yes, I misread the code, sorry.
johannes