Re: [Qemu-devel] [PATCH 4/5] NetRxPkt: Account buffer with ETH header in IOV length

[Dmitry Fleytman]

> On 3 Mar 2017, at 18:39 PM, Philippe Mathieu-Daudé <address@hidden> wrote:
> 
> Hi Dmitry,
> 
> On 02/16/2017 09:29 AM, Dmitry Fleytman wrote:
>> In case of VLAN stripping ETH header is stored in a
>> separate chunk and length of IOV should take this into
>> account.
>> 
>> This patch fixes checksum validation for RX packets
>> with VLAN header.
>> 
>> Devices affected by this problem: e1000e and vmxnet3.
>> 
>> Signed-off-by: Dmitry Fleytman <address@hidden>
>> ---
>> hw/net/net_rx_pkt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c
>> index d38babe..c7ae33d 100644
>> --- a/hw/net/net_rx_pkt.c
>> +++ b/hw/net/net_rx_pkt.c
>> @@ -97,7 +97,7 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt,
>>         pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len;
>>         pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1,
>>                                 iov, iovcnt, ploff,
>> -                                pkt->tot_len - pkt->ehdr_buf_len);
>> +                                pkt->tot_len - pkt->ehdr_buf_len) + 1;
> 
> I can not understand the trailing '1', shouldn't it be 'sizeof(struct 
> vlan_header)' instead? (=4)
> This should be easily unit-testable.

Hi Philippe,

Please pay attention that iov_copy returns number of items in target IOV,
pkt->vec_len is number of items as well, so +1 is perfectly fine here.

~Dmitry

> 
>>     } else {
>>         net_rx_pkt_iovec_realloc(pkt, iovcnt);