[SLOF] [PATCH v2 18/19] virtio-net: enable virtio 1.0

Nikunj A Dadhania nikunj at linux.vnet.ibm.com
Thu Jan 21 19:53:25 AEDT 2016


Thomas Huth <thuth at redhat.com> writes:

> On 20.01.2016 13:10, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>>  lib/libvirtio/virtio-net.c | 145 ++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 105 insertions(+), 40 deletions(-)
>> 
>> diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c
>> index aad7f9b..da9d224 100644
>> --- a/lib/libvirtio/virtio-net.c
>> +++ b/lib/libvirtio/virtio-net.c
> ...
>> @@ -273,14 +328,24 @@ static int virtionet_receive(char *buf, int maxlen)
>>  #endif
>>  
>>  	/* Copy data to destination buffer */
>> -	memcpy(buf, (void*)vq_rx.desc[id].addr, len);
>> +	if (virtiodev.is_modern)
>> +		memcpy(buf, (void*)le64_to_cpu(vq_rx.desc[id].addr), len);
>> +	else
>> +		memcpy(buf, (void*)vq_rx.desc[id].addr, len);
>>  
>>  	/* Move indices to next entries */
>>  	last_rx_idx = last_rx_idx + 1;
>>  
>> -	vq_rx.avail->ring[vq_rx.avail->idx % vq_rx.size] = id - 1;
>> -	sync();
>> -	vq_rx.avail->idx += 1;
>> +	if (virtiodev.is_modern) {
>> +		vq_rx.avail->ring[idx % vq_rx.size] = cpu_to_le16(id - 1);
>> +		sync();
>> +		vq_rx.avail->idx = cpu_to_le16(idx + 1);
>> +	}
>> +	else {
>> +		vq_rx.avail->ring[idx % vq_rx.size] = id - 1;
>> +		sync();
>> +		vq_rx.avail->idx += 1;
>> +	}
>>  
>>  	/* Tell HV that RX queue entry is ready */
>>  	virtio_queue_notify(&virtiodev, VQ_RX);
>
> You've now really got lots of these

I tried to reduce as much as possible. Trying to keep the core routine
intact and less churn. But these are inevitable ones,

>
>  if (is_modern)
>     somevar = cpu_to_le(someval);
>  else
>     somevar = someval;
>
> in here. Not so nice. Maybe you could introduce some wrappers, let's say
> cpu_to_modern16(dev, val), cpu_to_modern32(dev, val),
> modern_to_cpu16(dev, val) and modern_to_cpu32(dev, val) instead?
>
> Then the code would easily shrink to one-liners instead:
>
>    somevar = cpu_to_modern(dev, someval);
>
> What do you think?

A nice idea. Will implement in next rev.

Regards
Nikunj



More information about the SLOF mailing list