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

[Johannes Berg <johannes@sipsolutions.net>]

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


> > 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]