On Wednesday 23 January 2008 05:32:05 Luis R. Rodriguez wrote:
> On Jan 22, 2008 2:54 PM, Ivo van Doorn <ivdoorn@xxxxxxxxx> wrote:
> > Hi,
> >
> > > > > > Then there is a problem for rt2x00. Since the mactime isn't
> > > > > > known. rt2400pci is the _only_ device which has a RX_END_TIME
> > > > > > field in the RX descriptor.
> > > > >
> > > > > one workaround could be to simply use the current TSF at the time
> > > > > in the tasklet or interrupt handler (to be more close to the actual
> > > > > rx time). this should be sufficient to catch most cases where an
> > > > > IBSS merge is necessary - usually the beacon's TSF will be much
> > > > > higher than the local TSF.
> > > >
> > > > Should the driver to this, or should mac80211 handle that?
> > >
> > > The driver should if it has access to some the mactime of the received
> > > packet otherwise yes -- I think mac80211 can handle this using the
> > > supplied get_tsf().
> > >
> > > > Personally I think it is something for the mac80211 layer since the
> > > > driver will give what it can, and can be sure that it is what
> > > > mac80211 expects instead of drivers interpreting what mac80211 might
> > > > want as replacement. If mac80211 needs the TSF value when no mac time
> > > > is given, it could just use the get_tsf() callback function to the
> > > > driver to get the substitute. When the get_tsf() callback is not
> > > > provided, then mac80211 can complain about missing information.
well we can do that, but the closer you record the TSF of a packet after
reception, the better. this is why i suggested the interrupt handler or the
drivers rx tasklet. i don't know enough about when mac80211 rx handlers will
run, but it seems they could be delayed quite a bit (work queue?).
especially for IBSS merges we need to basically compare the TSF of the beaon
and the local TSF on a usec level. for most cases i guess getting the TSF in
ieee80211_rx_bss_info() should be sufficient, but it will not merge 100%
correctly if the time difference between IBSS nodes is small.
> I still think we should inform the user if the user switches to IBSS
> at some point, perhaps better during interface addition if their
> driver's IBSS mode is going to have some issues. How about we add to
> the enum ieee80211_hw_flags a "IEEE80211_HW_RX_MACTIME". Then we can
> warn accordingly during ieee80211_if_add() if the interface type is
> IBSS"
>
> * If driver supports IEEE80211_HW_RX_MACTIME we don't warn anything
> * If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is
> implemented inform user IBSS merge may not behave accurately
> * If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is not
> implemented warn IBSS merge will not work
jep, i think that would be best. can we add that in a separate patch?
> We could add:
>
> static inline u64 __approx_mactime(struct ieee80211_local *local) {
> BUG_ON(!local || !local->ops);
> return (local->ops->get_tsf) ?
> local->ops->get_tsf(local_to_hw(local)) : -1LLU;
> }
>
> Then in ieee80211_rx_bss_info() we can do something like:
>
> + if (local->hw.flags & IEEE80211_HW_RX_MACTIME) {
> + if (rx_status->flag & RX_FLAG_TSFT)
> + mactime = rx_status->mactime;
> + else {
> + WARN_ON(1);
> + mactime = __approx_mactime(local);
> + }
> + else {
> + mactime = __approx_mactime(local);
> + }
i'd prefer to do it without the inline just all in the function. like this:
+ if (rx_status->flag & RX_FLAG_TSFT)
+ /* in order for correct IBSS merging we need mactime*/
+ mactime = rx_status->mactime;
+ else if (local && local->ops && local->ops->get_tsf)
+ /* second best option: get current TSF */
+ mactime = local->ops->get_tsf(local_to_hw(local));
+ else
+ /* can't merge without knowing the TSF */
+ mactime = -1LLU;
comments? should i resend my patch?
bruno
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
