Re: [PATCH 1/1] ath9k: decrypt_error flag issue

[Pavel Roskin <proski@gnu.org>]

On Thu, 9 Aug 2012 18:21:59 +0530
Mohammed Shafi <shafi.wireless@gmail.com> wrote:

> > Assume hw reports a decryption error, the flag decrypt_error is set
> > to true in ath9k_rx_accept.
> > Since this flag is initialized to false just out of  ath_rx_tasklet
> > while cycle, all subsequent frames are marked as corrupted until
> > ath_rx_tasklet ends.
> 
> seems to be a nice catch!
> It would be great if we can simulate this situation (ie
> decrypt_error is set to true, and subsequent frames are discarded)
> may be we can try this some time, purposefully setting the
> decrypt_error 'true' for the first instance and see if there is data
> loss/throughput diff because of this.

That simulation might be too much work.  The only affected frames would
be those that are processed in the same tasklet as the frame with
decryption error.  What frame is processed in which tasklet is
timing-dependent.

It's enough to see that the code is wrong as it is.

ath9k_rx_accept() only sets decrypt_error to true and never to false.
It's not my favorite way of handling errors, but it's OK.

ath9k_rx_skb_preprocess() calls ath9k_rx_accept() and never writes or
reads decrypt_error in any other way.

ath_rx_tasklet() calls ath9k_rx_skb_preprocess() in a loop over the
frames to be processed.  Then it calls ath9k_rx_skb_postprocess() and
passes decrypt_error to it.  So we can have a leftover value from
another frame passed there.  In that case, the frame won't me marked as
decrypted.

I think it would be nice to examine all the variables initialized
outside the loop in ath_rx_tasklet().  There may be more catches
waiting to be discovered.

-- 
Regards,
Pavel Roskin