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

Thomas Huth thuth at redhat.com
Thu Jan 21 08:43:11 AEDT 2016


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

 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?

 Thomas




More information about the SLOF mailing list