[SLOF] [PATCH v1 26/27] virtio-net: make driver virtio 1.0 compatible
Nikunj A Dadhania
nikunj at linux.vnet.ibm.com
Thu Jan 14 22:38:40 AEDT 2016
Alexey Kardashevskiy <aik at ozlabs.ru> writes:
> On 01/13/2016 10:17 PM, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>> lib/libvirtio/virtio-net.c | 170 +++++++++++++++++++++++++++++----------------
>> 1 file changed, 111 insertions(+), 59 deletions(-)
>>
>> diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c
>> index 9c51a39..d789c38 100644
>> --- a/lib/libvirtio/virtio-net.c
>> +++ b/lib/libvirtio/virtio-net.c
>> @@ -42,7 +42,7 @@ struct virtio_device virtiodev;
>> struct vqs vq_rx; /* Information about virtqueues */
>> struct vqs vq_tx;
>>
>> -/* See Virtio Spec, appendix C, "Device Operation" */
>> +/* See Virtio Spec, appendix C, "Device Operation" */
>
> Unrelated change.
Sure, will change in separate patch
>
>
>> struct virtio_net_hdr {
>> uint8_t flags;
>> uint8_t gso_type;
>> @@ -55,6 +55,16 @@ struct virtio_net_hdr {
>>
>> static unsigned int net_hdr_size;
>>
>> +struct virtio_net_hdr_v1 {
>> + uint8_t flags;
>> + uint8_t gso_type;
>> + uint16_t hdr_len;
>> + uint16_t gso_size;
>> + uint16_t csum_start;
>> + uint16_t csum_offset;
>> + uint16_t num_buffers;
>
>
> Why not use the new "le16" type?
yes, will do that.
>
>
>> +};
>> +
>> static uint16_t last_rx_idx; /* Last index in RX "used" ring */
>>
>> /**
>> @@ -69,8 +79,7 @@ static int virtionet_init_pci(struct virtio_device *dev)
>> if (!dev)
>> return -1;
>>
>> - virtiodev.base = dev->base;
>> - virtiodev.type = dev->type;
>> + memcpy(&virtiodev, dev, sizeof(struct virtio_device));
>>
>> /* Reset device */
>> virtio_reset_device(&virtiodev);
>> @@ -90,23 +99,6 @@ static int virtionet_init_pci(struct virtio_device *dev)
>> return 0;
>> }
>>
>> -static void fill_desc(struct vring_desc *desc, uint32_t is_modern,
>> - uint64_t addr, uint32_t len,
>> - uint16_t flags, uint16_t next)
>> -{
>> - if (is_modern) {
>> - desc->addr = cpu_to_le64(addr);
>> - desc->len = cpu_to_le32(len);
>> - desc->flags = cpu_to_le16(flags);
>> - desc->next = cpu_to_le16(next);
>> - } else {
>> - desc->addr = addr;
>> - desc->len = len;
>> - desc->flags = flags;
>> - desc->next = next;
>> - }
>> -}
>> -
>> /**
>> * Initialize the virtio-net device.
>> * See the Virtio Spec, chapter 2.2.1 and Appendix C "Device Initialization"
>> @@ -115,6 +107,7 @@ static void fill_desc(struct vring_desc *desc, uint32_t is_modern,
>> static int virtionet_init(net_driver_t *driver)
>> {
>> int i;
>> + uint32_t status;
>>
>> dprintf("virtionet_init(%02x:%02x:%02x:%02x:%02x:%02x)\n",
>> driver->mac_addr[0], driver->mac_addr[1],
>> @@ -125,19 +118,27 @@ static int virtionet_init(net_driver_t *driver)
>> return 0;
>>
>> /* Tell HV that we know how to drive the device. */
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER);
>> + status = VIRTIO_STAT_ACKNOWLEDGE | VIRTIO_STAT_DRIVER;
>> + virtio_set_status(&virtiodev, status);
>>
>> /* Device specific setup - we do not support special features right now */
>> - virtio_set_guest_features(&virtiodev, 0);
>> + if (virtiodev.is_modern) {
>> + if (!virtio_negotiate_guest_features(&virtiodev, VIRTIO_F_VERSION_1 | (BIT(5))))
>> + goto dev_error;
>> + net_hdr_size = sizeof(struct virtio_net_hdr_v1);
>> + status |= VIRTIO_STAT_FEATURES_OK;
>> + virtio_set_status(&virtiodev, status);
>> + } else {
>> + net_hdr_size = sizeof(struct virtio_net_hdr);
>> + virtio_set_guest_features(&virtiodev, 0);
>> + }
>>
>> - net_hdr_size = sizeof(struct virtio_net_hdr);
>> /* Allocate memory for one transmit an multiple receive buffers */
>> vq_rx.buf_mem = SLOF_alloc_mem((BUFFER_ENTRY_SIZE+net_hdr_size)
>> * RX_QUEUE_SIZE);
>> if (!vq_rx.buf_mem) {
>> printf("virtionet: Failed to allocate buffers!\n");
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_FAILED);
>> - return -1;
>> + goto dev_error;
>> }
>>
>> /* Prepare receive buffer queue */
>> @@ -146,35 +147,53 @@ static int virtionet_init(net_driver_t *driver)
>> + i * (BUFFER_ENTRY_SIZE+net_hdr_size);
>> uint32_t id = i*2;
>> /* Descriptor for net_hdr: */
>> - fill_desc(&vq_rx.desc[id], 0, addr, net_hdr_size,
>> + virtio_fill_desc(&vq_rx.desc[id], virtiodev.is_modern, addr, net_hdr_size,
>> VRING_DESC_F_NEXT | VRING_DESC_F_WRITE, id + 1);
>>
>> /* Descriptor for data: */
>> - fill_desc(&vq_rx.desc[id+1], 0, addr + net_hdr_size,
>> + virtio_fill_desc(&vq_rx.desc[id+1], virtiodev.is_modern, addr + net_hdr_size,
>> BUFFER_ENTRY_SIZE, VRING_DESC_F_WRITE, 0);
>>
>> - vq_rx.avail->ring[i] = id;
>> + if (virtiodev.is_modern)
>> + vq_rx.avail->ring[i] = cpu_to_le16(id);
>> + else
>> + vq_rx.avail->ring[i] = id;
>> }
>> sync();
>> - vq_rx.avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> - vq_rx.avail->idx = RX_QUEUE_SIZE;
>> + if (virtiodev.is_modern) {
>> + vq_rx.avail->flags = cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
>> + vq_rx.avail->idx = cpu_to_le16(RX_QUEUE_SIZE);
>>
>> - last_rx_idx = vq_rx.used->idx;
>> + last_rx_idx = le16_to_cpu(vq_rx.used->idx);
>> +
>> + vq_tx.avail->flags = cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
>> + vq_tx.avail->idx = 0;
>> + } else {
>> + vq_rx.avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> + vq_rx.avail->idx = RX_QUEUE_SIZE;
>>
>> - vq_tx.avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> - vq_tx.avail->idx = 0;
>> + last_rx_idx = vq_rx.used->idx;
>>
>> + vq_tx.avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
>> + vq_tx.avail->idx = 0;
>> + }
>> /* Tell HV that setup succeeded */
>> - virtio_set_status(&virtiodev, VIRTIO_STAT_ACKNOWLEDGE
>> - |VIRTIO_STAT_DRIVER
>> - |VIRTIO_STAT_DRIVER_OK);
>> + status |= VIRTIO_STAT_DRIVER_OK;
>> + virtio_set_status(&virtiodev, status);
>>
>> /* Tell HV that RX queues are ready */
>> virtio_queue_notify(&virtiodev, VQ_RX);
>>
>> driver->running = 1;
>> -
>> + for(i=0;i<6;i++) {
>
>
> s/6/sizeof(driver->mac_addr)/ ?
Sure.
>
>
>> + driver->mac_addr[i] = virtio_get_config(&virtiodev, i, 1);
>> + }
>> return 0;
>> +dev_error:
>> + driver->running = 0;
>> + status |= VIRTIO_STAT_FAILED;
>> + virtio_set_status(&virtiodev, status);
>> + return -1;
>> }
>>
>>
>> @@ -207,7 +226,7 @@ static int virtionet_term(net_driver_t *driver)
>> */
>> static int virtionet_xmit(char *buf, int len)
>> {
>> - int id;
>> + int id, idx;
>> static struct virtio_net_hdr nethdr;
>>
>> if (len > BUFFER_ENTRY_SIZE) {
>> @@ -220,20 +239,32 @@ static int virtionet_xmit(char *buf, int len)
>> memset(&nethdr, 0, sizeof(nethdr));
>>
>> /* Determine descriptor index */
>> - id = (vq_tx.avail->idx * 2) % vq_tx.size;
>> + if (virtiodev.is_modern)
>> + idx = le16_to_cpu(vq_tx.avail->idx);
>> + else
>> + idx = vq_tx.avail->idx;
>> + id = (idx * 2) % vq_tx.size;
>>
>> /* Set up virtqueue descriptor for header */
>> - fill_desc(&vq_tx.desc[id], 0, (uint64_t)&nethdr,
>> + virtio_fill_desc(&vq_tx.desc[id], virtiodev.is_modern, (uint64_t)&nethdr,
>> net_hdr_size, VRING_DESC_F_NEXT, id + 1);
>>
>> /* Set up virtqueue descriptor for data */
>> - fill_desc(&vq_tx.desc[id+1], 0, (uint64_t)buf, len, 0, 0);
>> + virtio_fill_desc(&vq_tx.desc[id+1], virtiodev.is_modern, (uint64_t)buf, len, 0, 0);
>>
>> - vq_tx.avail->ring[vq_tx.avail->idx % vq_tx.size] = id;
>> - sync();
>> - vq_tx.avail->idx += 1;
>> - sync();
>> + if (virtiodev.is_modern) {
>> + vq_tx.avail->ring[idx % vq_tx.size] = cpu_to_le16(id);
>> + sync();
>> + vq_tx.avail->idx = cpu_to_le16(idx + 1);
>> + sync();
>> + } else {
>> + vq_tx.avail->ring[idx % vq_tx.size] = id;
>> + sync();
>> + vq_tx.avail->idx += 1;
>> + sync();
>> + }
>>
>> + dprintf("virtionet_xmit: id %d, idx %d\n", id, idx);
>> /* Tell HV that TX queue is ready */
>> virtio_queue_notify(&virtiodev, VQ_TX);
>>
>> @@ -246,19 +277,31 @@ static int virtionet_xmit(char *buf, int len)
>> */
>> static int virtionet_receive(char *buf, int maxlen)
>> {
>> - int len = 0;
>> - int id;
>> + uint32_t len = 0;
>> + uint32_t id, idx;
>>
>> - if (last_rx_idx == vq_rx.used->idx) {
>> + if (virtiodev.is_modern)
>> + idx = le16_to_cpu(vq_rx.used->idx);
>> + else
>> + idx = vq_rx.used->idx;
>> +
>> + if (last_rx_idx == idx) {
>> /* Nothing received yet */
>> return 0;
>> }
>>
>> - id = (vq_rx.used->ring[last_rx_idx % vq_rx.size].id + 1)
>> - % vq_rx.size;
>> - len = vq_rx.used->ring[last_rx_idx % vq_rx.size].len
>> - - net_hdr_size;
>> -
>> + if (virtiodev.is_modern) {
>> + id = (le32_to_cpu(vq_rx.used->ring[last_rx_idx % vq_rx.size].id) + 1)
>> + % vq_rx.size;
>> + len = le32_to_cpu(vq_rx.used->ring[last_rx_idx % vq_rx.size].len)
>> + - net_hdr_size;
>> + dprintf("%p id %x len %x\n", vq_rx.used, vq_rx.used->ring[last_rx_idx % vq_rx.size].id, vq_rx.used->ring[last_rx_idx % vq_rx.size].len);
>> + } else {
>> + id = (vq_rx.used->ring[last_rx_idx % vq_rx.size].id + 1)
>> + % vq_rx.size;
>> + len = vq_rx.used->ring[last_rx_idx % vq_rx.size].len
>> + - net_hdr_size;
>> + }
>> dprintf("virtionet_receive() last_rx_idx=%i, vq_rx.used->idx=%i,"
>> " id=%i len=%i\n", last_rx_idx, vq_rx.used->idx, id, len);
>>
>> @@ -276,19 +319,28 @@ static int virtionet_receive(char *buf, int maxlen)
>> if ((i%16)==15)
>> printf("\n");
>> }
>> - prinfk("\n");
>> + printf("\n");
>
>
> Unrelated change, deserves a separate patch in the beginning of the patchset.
>
>
>> #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);
>>
>>
>
>
> --
> Alexey
More information about the SLOF
mailing list