Re: [dpdk-stable] [dpdk-dev] [PATCH] net/virtio: fix mbuf data and pkt length mismatch

[Andrew Rybchenko <arybchenko@solarflare.com>]

Hi Tiwei,

On 10/15/19 8:33 AM, Tiwei Bie wrote:
> On Mon, Oct 14, 2019 at 06:15:42PM +0300, Andrew Rybchenko wrote:
>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>> index 27ead19fb..822cce06d 100644
>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>> @@ -597,9 +597,8 @@ virtqueue_enqueue_xmit_inorder(struct virtnet_tx *txvq,
>>>    		dxp->cookie = (void *)cookies[i];
>>>    		dxp->ndescs = 1;
>>> -		hdr = (struct virtio_net_hdr *)
>>> -			rte_pktmbuf_prepend(cookies[i], head_size);
>>> -		cookies[i]->pkt_len -= head_size;
>>> +		hdr = (struct virtio_net_hdr *)(char *)cookies[i]->buf_addr +
>>> +			cookies[i]->data_off - head_size;
>>>    		/* if offload disabled, hdr is not zeroed yet, do it now */
>>>    		if (!vq->hw->has_tx_offload)
>>> @@ -608,9 +607,10 @@ virtqueue_enqueue_xmit_inorder(struct virtnet_tx *txvq,
>>>    			virtqueue_xmit_offload(hdr, cookies[i], true);
>>>    		start_dp[idx].addr  = VIRTIO_MBUF_DATA_DMA_ADDR(cookies[i], vq);
>> As I understand the problem is here. It points to start of the packet
>> (Ethernet header) since data_off is not changed above now, but
>> should point to virtio_net_hdr before the packet.
>>
>> I think the patch fixes the bug in a wrong direction. It looks better
>> to simply remove
>>
>>      cookies[i]->pkt_len -= head_size;
>>
>> above and care about real packet length difference in
>> virtio_update_packet_stats() or when it is called from Tx path.
>>
>> If it is OK for maintainers I'm ready to send patches to rollback back
>> this one and fix it as described above.
> Hi Andrew,
>
> Thanks for looking into this! Feel free to send your fix.
> PS. Another thing also needs to be noticed is that, after
> prepending the net hdr with rte_pktmbuf_prepend(), below
> code in virtio_update_packet_stats() won't be able to
> access the ether header as expected:
>
> https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_rxtx.c#L134-L140

Thanks for the information. Looking closer I've changed my mind.
Will send patch shortly.

Andrew.