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

[Vladimir Oltean <vladimir.oltean@nxp.com>]

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?