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

[Zhang Chen <zhangchen.fnst@cn.fujitsu.com>]

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 
> <jasowang@redhat.com>" 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


> Thanks
>
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   include/net/net.h |  4 +++-
>>   net/net.c         | 24 ++++++++++++++++++++++--
>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 402d913..865cb98 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -115,9 +115,11 @@ typedef struct NICState {
>>   } NICState;
>>     struct SocketReadState {
>> -    int state; /* 0 = getting length, 1 = getting data */
>> +    /* 0 = getting length, 1 = getting vnet header length, 2 = 
>> getting data */
>> +    int state;
>>       uint32_t index;
>>       uint32_t packet_len;
>> +    uint32_t vnet_hdr_len;
>>       uint8_t buf[NET_BUFSIZE];
>>       SocketReadStateFinalize *finalize;
>>   };
>> diff --git a/net/net.c b/net/net.c
>> index f69260f..5a6b5ac 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const 
>> uint8_t *buf, int size)
>>       unsigned int l;
>>         while (size > 0) {
>> -        /* reassemble a packet from the network */
>> -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
>> +        /* Reassemble a packet from the network.
>> +         * 0 = getting length.
>> +         * 1 = getting vnet header length.
>> +         * 2 = getting data.
>> +         */
>> +        switch (rs->state) {
>>           case 0:
>>               l = 4 - rs->index;
>>               if (l > size) {
>> @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const 
>> uint8_t *buf, int size)
>>               }
>>               break;
>>           case 1:
>> +            l = 4 - rs->index;
>> +            if (l > size) {
>> +                l = size;
>> +            }
>> +            memcpy(rs->buf + rs->index, buf, l);
>> +            buf += l;
>> +            size -= l;
>> +            rs->index += l;
>> +            if (rs->index == 4) {
>> +                /* got vnet header length */
>> +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
>> +                rs->index = 0;
>> +                rs->state = 2;
>> +            }
>> +            break;
>> +        case 2:
>>               l = rs->packet_len - rs->index;
>>               if (l > size) {
>>                   l = size;
>
>
>
> .
>

-- 
Thanks
Zhang Chen