[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76c94541-21a8-7ae5-c4c4-48552f16c3fd@suse.com>
Date: Thu, 25 Feb 2021 08:33:41 +0100
From: Jan Beulich <jbeulich@...e.com>
To: paul@....org
Cc: "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Wei Liu <wl@....org>
Subject: Re: [PATCH] xen-netback: correct success/error reporting for the
SKB-with-fraglist case
On 24.02.2021 17:39, Paul Durrant wrote:
> On 23/02/2021 16:29, Jan Beulich wrote:
>> When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
>> special considerations for the head of the SKB no longer apply. Don't
>> mistakenly report ERROR to the frontend for the first entry in the list,
>> even if - from all I can tell - this shouldn't matter much as the overall
>> transmit will need to be considered failed anyway.
>>
>> Signed-off-by: Jan Beulich <jbeulich@...e.com>
>>
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -499,7 +499,7 @@ check_frags:
>> * the header's copy failed, and they are
>> * sharing a slot, send an error
>> */
>> - if (i == 0 && sharedslot)
>> + if (i == 0 && !first_shinfo && sharedslot)
>> xenvif_idx_release(queue, pending_idx,
>> XEN_NETIF_RSP_ERROR);
>> else
>>
>
> I think this will DTRT, but to my mind it would make more sense to clear
> 'sharedslot' before the 'goto check_frags' at the bottom of the function.
That was my initial idea as well, but
- I think it is for a reason that the variable is "const".
- There is another use of it which would then instead need further
amending (and which I believe is at least part of the reason for
the variable to be "const").
Jan
Powered by blists - more mailing lists