Re: [Qemu-devel] [PATCH v1 2/5] cadence_gem: Correct the multi-queue can rx logic

[Peter Maydell]

On 5 April 2017 at 00:40, Alistair Francis <address@hidden> wrote:
> Correct the buffer descriptor busy logic to work correctly when using
> multiple queues.
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>
>  hw/net/cadence_gem.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 17c229d..3e37665 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -481,14 +481,18 @@ static int gem_can_receive(NetClientState *nc)
>      }
>
>      for (i = 0; i < s->num_priority_queues; i++) {
> -        if (rx_desc_get_ownership(s->rx_desc[i]) == 1) {
> -            if (s->can_rx_state != 2) {
> -                s->can_rx_state = 2;
> -                DB_PRINT("can't receive - busy buffer descriptor (q%d) 
> 0x%x\n",
> -                         i, s->rx_desc_addr[i]);
> -             }
> -            return 0;
> +        if (rx_desc_get_ownership(s->rx_desc[i]) != 1) {
> +            break;
> +        }
> +    };
> +
> +    if (i == s->num_priority_queues) {
> +        if (s->can_rx_state != 2) {
> +            s->can_rx_state = 2;
> +            DB_PRINT("can't receive - busy buffer descriptor (q%d) 0x%x\n",
> +                     i, s->rx_desc_addr[i]);

This looks a little odd -- surely i isn't the right index to use
into rx_desc_addr[] any more now we're outside the loop and i
is always larger than the largest valid queue number? It looks
like the debug print should be rephrased somehow.

thanks
-- PMM