Re: [PATCH 5/8] mac80211: A-MPDU Rx handling aggregation reordering

["Ron Rindjunsky" <rindjon@googlemail.com>]

ok, I agree.
I'll make one patch for seperation only inside __ieee80211_rx, and
then build the reordering on top of it.

> > > Also, what you're doing means that
> > > the frame is accounted for channel load twice (yeah, I know, the stats
> > > there suck... but still...)
> >
> > here i am already checking the .ordered field (like i proposed for
> > monitor), and ignore the in-order frames.
>
> Ah. You'd need to ignore the out-of-order frames too since they too have
> gone through __ieee80211_rx already, no? Or do you by out-of-order just
> mean those that have been queued?
>
> > so you suggest to remove ieee80211_rx_h_reorder_ampdu in favor of a
> > function that will call ieee80211_sta_manage_reorder_buf?
>
> I guess the rx_h_reorder_ampdu pre-handler can stay, but as far as I
> understood the flow it will cause some frames to be "buffered". And I
> think frames shouldn't go through the same path twice even if you've
> annotated that path all over skipping when the frame was injected.
>
> Basically, it seems to me that you're injecting frames down the same
> __ieee80211_rx() path when they have already gone through that, and then
> after the fact annotate pretty much all of that path before the actual
> rx handlers (after pre-handlers) with "skip if reordered frame". That I
> don't really like, it means we need to special-case everything we ever
> put there.
>
> > 2 - (pro) yet, i'll end up checking the .ordered field in 3 places -
> > monitor,statistics and reordering buf, which lowers the encapsulation
> > of work inside ieee80211_rx_h_reorder_ampdu
> > 3 - (con) i will have to free un-needed sk_buffs in
> > ieee80211_sta_stop_rx_BA_session without going back to __ieee80211_rx,
> >  which may confuse othere.
>
> I don't think it's that confusing because __ieee80211_rx would do the
> same thing, it would free the frame if requested by the pre-handlers
> instead of passing it to the rx handlers. What your code does would
> simply be a diversion through another queue that has the same effect.
> (If you keep the rx-pre-handler for that you should add a comment that
> it must be last)
>
> johannes
>
>