qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to S


From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
Date: Tue, 9 May 2017 10:40:45 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0



On 2017年05月08日 11:47, Zhang Chen wrote:


On 05/03/2017 06:42 PM, Jason Wang wrote:


On 2017年05月03日 11:43, Zhang Chen wrote:


On 05/02/2017 12:53 PM, Jason Wang wrote:


On 2017年04月28日 17:47, Zhang Chen wrote:
Address Jason Wang's comments add vnet header length to SocketReadState.

Instead of saying this, you can add "Suggested-by: Jason Wang <address@hidden>" above your sign-off.

OK.


So we change net_fill_rstate() to read
struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.

This makes me thinking about the backward compatibility. I think we'd better try to keep it as much as possible. E.g how about pack vnet_hdr_len into higher bits of size?


Do you means split uint32_t size to uint16_t size and uint16_t vnet_hdr_len ?
If yes, we also can't keep compatibility with old version.
Old code send a uint32_t size, we read it as uint16_t size is always wrong.

Thanks
Zhang Chen

Consider it's unlikely to send or receive packet >= 64K, we can reuse higher bits (e.g highest 8 bits). Then we can still read uint32_t and then check its highest 8 bits. If it was zero, we know vnet header is zero, if not it could be read as vnet header length.

I got your point, but in this way, packet size must < 64K, if size >=64K we still can't maintain the backward compatibility, For the packet sender that didn't know the potential packet size limit, I think we should make code explicitly declared like currently code. Otherwise we will make other people confused and make code difficult to maintain.

Thanks
Zhang Chen


Yes, this is an possible issue in the future. Rethink about this, what if we just compare vnet header? Is there any issue of doing this? (Sorry, I remember this is a reason, but forget now).

Thanks



reply via email to

[Prev in Thread] Current Thread [Next in Thread]