Re: [PATCH rfc v0 2/9] net: dsa: qca8k: Move completion into DSA core

On Wed, Sep 21, 2022 at 02:19:54AM +0200, Andrew Lunn wrote:

I've been thinking about this a bit. And i think the bug is in the old
code.

As soon as we call reinit_completion(), a late arriving packet can
trigger the completion. Note that the sequence number has not been
incremented yet. So that late arriving packet can pass the sequence
number test, and the results will be copied and complete() called.

qca8k_read_eth() can continue, increment the sequence number, call
wait_for_completion_timeout() and immediately exit, returning the
contents of the late arriving reply.

To make this safe:

1) The sequence number needs to be incremented before
   reinit_completion(). That closes one race

2) If the sequence numbers don't match, silently drop the packet, it
   is either later, or bogus. Hopefully the correct reply packet will
   come along soon and trigger the completion.

I've also got some of this wrong in my code.

Wouldn't the programming be more obvious if we didn't try to reuse the
same dsa_inband structure for every request/response, but simply
allocate/free on demand and use only once?