Hi Ron, > Johannes, i re-inspected the code to implement your solution, and it > is not straight forward as you have seen it. Ok. > what we do now in __ieee80211_rx is: > - invoke monitor > - fill in txrx_data > - invoke michael_mic_report (if needed) > - call pre-handlers of QoS that change txrx_data as well. > - call pre-handlers of statistics that change txrx_data > - call reorder buffer prehandler > - saving sk_buff to reordering buffer. > - if we can, send sk_buffs back to __ieee80211_rx, that will > properly fill txrx_data Yeah, sounds like it. Some things like monitor may discard the frame. > now, in order to proceed according to your implementation, as i do not > call again __ieee80211, and do not get the txrx_data restored, i > will either: > 1 - have to fill in txrx_data inside reordering function, which will > end in massive code duplication, and i don't like this solution > 2 - keep the whole txrx_data and not only the sk_buff inside > reordering buffer, which will end up in allocation of the txrx_data > struct for every incoming frame, or in a big maximum possible size of > 64 pre-allocated array of txrx_data per tid - this is either waste of > cpu or memory Hmm, good points. Didn't really think about it but yeah, txrx data is on the stack there. > so, what do you think about next solution: > > 1 - move the rxtx_data indifferent functionality out of __ieee80211_rx > (monitor, load and reordering) to, lets say __ieee80211_rx_pre_handle. Sounds fine. > 2 - __ieee80211_rx_pre_handle will be called for each frame. either > directly or through tasklet. Hm, ok, we should keep __ieee80211_rx() as the entry point though so we don't have to change drivers. > 3 - in regular state, proceed to __ieee80211_rx immediately, in > reorder state loop over __ieee80211_rx for each already ordered frame. Ah ok. > 4 - I thought to eliminate __ieee80211_invoke_rx_handlers(local, > local->rx_pre_handlers), as QoS will be the only one to inhabit it > now, so it may move to ieee80211_invoke_rx_handlers(local, > local->rx_handlers) I don't think it can move there because the rx handlers can potentially be invoked multiple times. Will that work? In any case, I'm not against totally dissolving the pre-handlers into function calls. Makes for better code generation anyway since gcc will be able to optimise across the functions when inlining them (since they'll all have only one user) > sorry for the long mail, but i wanted to write my full opinion about this issue. Not at all, thanks for doing that. I think we should probably keep __iee80211_rx() as the entry point for the tasklet or the drivers. So I'd see the flow as: tasklet/driver __ieee80211_rx() __rx_pre_monitor() __rx_pre_load() __rx_pre_qos() if (agg) __rx_handle_reorder() else __rx_handle_packet() and __rx_handle_reorder() calls __rx_handle_packet() which does all the rest, I guess. Does that sound sane to you? You're in a better position to judge. I think this is basically what you wanted, just keeping __ieee80211_rx() as the main entry point. Or maybe I'm totally off. Can you explain again what we need to do for such a reordered frame? johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
