Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[Derek Smithies]

Hi,

On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:

> On Jan 18, 2008 7:50 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> > +                * always extend the mac timestamp, since this information is
> > +                * also needed for proper IBSS merging.
> > +                *
> > +                * XXX: it might be too late to do it here, since rs_tstamp is
> > +                * 15bit only. that means TSF extension has to be done within
> > +                * 32.768usec = 32ms. it might be necessary to move this to the
> > +                * interrupt handler, like it is done in madwifi.
> > +                */
> 
> I'm trying to understand this a bit more and am confused. The TSF
> timer is based on 1MHz clock so it has a resolution of 1 microsecond
> (us). For 15 bits that would mean 32768 us so don't you mean it should
> be done within 32.768 ms instead of usec (or us)?
> 
Hi,
 it is a typo.

You are correct. It should be done within 32.768 ms. On a high end laptop, 
it is almost guaranteed that the bottom half will process the packet 
within 32 ms. However, on an embedded box (low spec cpu) that has a 
temporarily high load, 32 ms is a short time, and the timestamp may have 
wrapped around. this is unfortunate.

Derek.
-- 
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
Email: derek@indranet.co.nz
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[bruno randolf]

On Sunday 20 January 2008 11:12:11 Derek Smithies wrote:
> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > On Jan 18, 2008 7:50 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* always extend the mac timestam=
p, since this
> > > information is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* also needed for=
 proper IBSS merging.
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* XXX: it might be too late to d=
o it here, since
> > > rs_tstamp is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 15bit only. that =
means TSF extension
> > > has to be done within + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 32.768us=
ec =3D 32ms. it might be
> > > necessary to move this to the + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =
interrupt handler,
> > > like it is done in madwifi. + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> >
> > I'm trying to understand this a bit more and am confused. The TSF
> > timer is based on 1MHz clock so it has a resolution of 1 microsecon=
d
> > (us). For 15 bits that would mean 32768 us so don't you mean it sho=
uld
> > be done within 32.768 ms instead of usec (or us)?
>
> Hi,
> =A0it is a typo.

it's just a different way to use a "." to denote 1000. americans might
write 32,768usec, im not sure, different styles worldwide... what i mea=
n
is 32768us equals about 32ms. i'll remove that dot to make it unambigou=
s.

> You are correct. It should be done within 32.768 ms. On a high end la=
ptop,
> it is almost guaranteed that the bottom half will process the packet
> within 32 ms. However, on an embedded box (low spec cpu) that has a
> temporarily high load, 32 ms is a short time, and the timestamp may h=
ave
> wrapped around. this is unfortunate.

i know that this might happen, and that's why i included this comment. =
i
was just too lazy to make the same act as is done in madwifi (lopping t=
hru
all rx descriptors in the interrupt handler). the current code is
sufficient for IBSS testing right now.

also even if we move TSF extension into the interrupt handler this will
not help in all cases. there are situations in IBSS mode - when a HW me=
rge
(=3D automatic HW TSF update upon receipt of a beacon with a higherr TS=
=46 and
the same BSSID) happens - where the rx timestamp is apparently based on
the old TSF, before the HW TSF is updated. in that case we cannot exten=
d
the timestamp in any meaningful way. i'm not sure how we should handle
this.

> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> Right now we only use mactime and even RX_FLAG_TSFT within mac80211 i=
n
> rx.c on ieee80211_rx_monitor() for IEEE80211_RADIOTAP_TSFT so right
> now these changes would seem to do nothing. Should we be using this
> for something else -- if so what is it?

see my "[PATCH] mac80211: enable IBSS merging". it is used there to dec=
ide
wether we have to merge IBSS on receipt of a beacon. strictly speaking =
it
would be sufficient to extend the rxstamp only for beacons and in monit=
or
mode, but i thought checking for these cases is more overhead, so why n=
ot
extend TSF all the time...

bruno

-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[Luis R. Rodriguez]

On Jan 20, 2008 12:37 AM, bruno randolf <br1@einfach.org> wrote:
> On Sunday 20 January 2008 11:12:11 Derek Smithies wrote:
> > On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > > On Jan 18, 2008 7:50 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> > > > + * always extend the mac timestamp, since this
> > > > information is + * also needed for proper IBSS merging.
> > > > + *
> > > > + * XXX: it might be too late to do it here, since
> > > > rs_tstamp is + * 15bit only. that means TSF extension
> > > > has to be done within + * 32.768usec = 32ms. it might be
> > > > necessary to move this to the + * interrupt handler,
> > > > like it is done in madwifi. + */
> > >
> > > I'm trying to understand this a bit more and am confused. The TSF
> > > timer is based on 1MHz clock so it has a resolution of 1 microsecond
> > > (us). For 15 bits that would mean 32768 us so don't you mean it should
> > > be done within 32.768 ms instead of usec (or us)?
> >
> > Hi,
> > it is a typo.
>
> it's just a different way to use a "." to denote 1000. americans might
> write 32,768usec, im not sure, different styles worldwide...

Yes, that is the 'american' way, not that it is correct.

> what i mean
> is 32768us equals about 32ms. i'll remove that dot to make it unambigous.

Yeah thanks, might be more clear for those of us not used to that
convention, I didn't even know it existed. Where is this "." for 1000
notation convention used, just so I know :) ?

> > You are correct. It should be done within 32.768 ms. On a high end laptop,
> > it is almost guaranteed that the bottom half will process the packet
> > within 32 ms. However, on an embedded box (low spec cpu) that has a
> > temporarily high load, 32 ms is a short time, and the timestamp may have
> > wrapped around. this is unfortunate.
>
> i know that this might happen, and that's why i included this comment. i
> was just too lazy to make the same act as is done in madwifi (lopping thru
> all rx descriptors in the interrupt handler). the current code is
> sufficient for IBSS testing right now.

Thanks for the explanation.

BTW while on the topic of TSF extension, anyone get why we do the tsf
-= 0x8000 if the hw's tsf's 15 bits value is < the rx descriptor's
rstamp? This is what ath5k_extend_tsf() does right before the (tsf &
~0x7fff) | rstamp.

> also even if we move TSF extension into the interrupt handler this will
> not help in all cases. there are situations in IBSS mode - when a HW merge
> (= automatic HW TSF update upon receipt of a beacon with a higherr TSF and
> the same BSSID) happens - where the rx timestamp is apparently based on
> the old TSF, before the HW TSF is updated. in that case we cannot extend
> the timestamp in any meaningful way. i'm not sure how we should handle
> this.

Hmm.. have you seen this with madwifi driver? What do you think is the
cause? If we don't do proper locking I can see this being an issue on
the driver side unless this is specifically a hardware issue.

> > On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > Right now we only use mactime and even RX_FLAG_TSFT within mac80211 in
> > rx.c on ieee80211_rx_monitor() for IEEE80211_RADIOTAP_TSFT so right
> > now these changes would seem to do nothing. Should we be using this
> > for something else -- if so what is it?
>
> see my "[PATCH] mac80211: enable IBSS merging". it is used there to decide
> wether we have to merge IBSS on receipt of a beacon.

Thanks, I have started to review it.

> strictly speaking it
> would be sufficient to extend the rxstamp only for beacons and in monitor
> mode, but i thought checking for these cases is more overhead, so why not
> extend TSF all the time...

Well sure, and also what's the point in updating mactime with 15-bit
value? So yes, sure, lets just keep it. Monitor mode and promiscuous
mode could use it as well. The only real penalty is in STA for
non-beacon frames and that is a matter of how expensive
ath5k_hw_get_tsf64() is and that's just two register reads.

  Luis

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[bruno randolf]

On Sunday 20 January 2008 17:46:05 Luis R. Rodriguez wrote:
> Yeah thanks, might be more clear for those of us not used to that
> convention, I didn't even know it existed. Where is this "." for 1000
> notation convention used, just so I know :) ?

in german speaking countries at least (i am austrian), don't really know
where else...

> > > You are correct. It should be done within 32.768 ms. On a high end
> > > laptop, it is almost guaranteed that the bottom half will process the
> > > packet within 32 ms. However, on an embedded box (low spec cpu) that
> > > has a temporarily high load, 32 ms is a short time, and the timestamp
> > > may have wrapped around. this is unfortunate.
> >
> > i know that this might happen, and that's why i included this comment. i
> > was just too lazy to make the same act as is done in madwifi (lopping
> > thru all rx descriptors in the interrupt handler). the current code is
> > sufficient for IBSS testing right now.
>
> Thanks for the explanation.
>
> BTW while on the topic of TSF extension, anyone get why we do the tsf
> -= 0x8000 if the hw's tsf's 15 bits value is < the rx descriptor's
> rstamp? This is what ath5k_extend_tsf() does right before the (tsf &
> ~0x7fff) | rstamp.

this is in order to account for situations where the last 15 bit of the
TSF have wrapped around in the mean time, since the rstamp was recorded
with the packet. it is not possible that we received the packet *after*
the current TSF (in the interrupt handler or in the tasklet), so we assume
that the 15bit value has overflown and therefore the time at packet
reception must have been - 0x8000.

> > also even if we move TSF extension into the interrupt handler this will
> > not help in all cases. there are situations in IBSS mode - when a HW
> > merge (= automatic HW TSF update upon receipt of a beacon with a higherr
> > TSF and the same BSSID) happens - where the rx timestamp is apparently
> > based on the old TSF, before the HW TSF is updated. in that case we
> > cannot extend the timestamp in any meaningful way. i'm not sure how we
> > should handle this.
>
> Hmm.. have you seen this with madwifi driver? What do you think is the
> cause? If we don't do proper locking I can see this being an issue on
> the driver side unless this is specifically a hardware issue.

i have seen this in ath5k and other people have seen it in madwifi.
therefore i'm pretty sure it's a HW issue. unfortunately this happens with
ad-hoc beacons where it's most critical to get a correct TSF.

bruno

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[Jiri Slaby]

On 01/20/2008 10:53 AM, bruno randolf wrote:
> On Sunday 20 January 2008 17:46:05 Luis R. Rodriguez wrote:
>> Yeah thanks, might be more clear for those of us not used to that
>> convention, I didn't even know it existed. Where is this "." for 1000
>> notation convention used, just so I know :) ?
> 
> in german speaking countries at least (i am austrian), don't really know
> where else...

In Czechia too (few (hundreds) km northern/eastern). So central europe, at least ;).

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

[Kalle Valo]

Luis R. Rodriguez <mcgrof@winlab.rutgers.edu> writes:

>> what i mean
>> is 32768us equals about 32ms. i'll remove that dot to make it unambigous.
>
> Yeah thanks, might be more clear for those of us not used to that
> convention, I didn't even know it existed. Where is this "." for 1000
> notation convention used, just so I know :) ?

It seems most of Europe uses it, even though we Finns don't:

"In much of Europe, however, a comma is used as a decimal separator,
while a full stop or a space is used for the presentation of large
numbers:

        * 1.000.000 (One million)
        * 1.000,000 or 1 000,000 (One thousand)"

http://en.wikipedia.org/wiki/Full_stop#Mathematical_usage

-- 
Kalle Valo