Re: [dpdk-dev] [PATCH] virtio: fix used ring address calculation

["Xie, Huawei" <huawei.xie@intel.com>]

On 9/25/2015 12:36 AM, Stephen Hemminger wrote:
> On Thu, 24 Sep 2015 07:30:41 +0000
> "Xie, Huawei" <huawei.xie@intel.com> wrote:
>
>> On 9/21/2015 11:39 AM, Xie, Huawei wrote:
>> vring_size calculation should consider both used_event_idx at the tail
>> of avail ring and avail_event_idx at the tail of used ring.
>> Will merge those two fixes and send a new patch.
>>> used event idx is put at the end of available ring. It isn't taken into account
>>> when we calculate the address of used ring. Fortunately, it doesn't introduce
>>> the bug with fixed queue number 256 and 4KB alignment.
>>>
>>> Signed-off-by: hxie5 <huawei.xie@intel.com>
>>> ---
>>>  drivers/net/virtio/virtio_ring.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h
>>> index a16c499..92e430d 100644
>>> --- a/drivers/net/virtio/virtio_ring.h
>>> +++ b/drivers/net/virtio/virtio_ring.h
>>> @@ -145,7 +145,7 @@ vring_init(struct vring *vr, unsigned int num, uint8_t *p,
>>>  	vr->avail = (struct vring_avail *) (p +
>>>  		num * sizeof(struct vring_desc));
>>>  	vr->used = (void *)
>>> -		RTE_ALIGN_CEIL((uintptr_t)(&vr->avail->ring[num]), align);
>>> +		RTE_ALIGN_CEIL((uintptr_t)(&vr->avail->ring[num + 1]), align);
>>>  }
>>>  
>>>  /*
> Why aren't we just using the standard Linux includes for this?
> See <linux/virtio_ring.h> and the function vring_init()
>
> Keeping parallel copies of headers is prone to failures.
Agree.
Using standard Linux includes then at least we don't need to redefine
the feature and other related MACRO.
This applies to vhost as well.
For vring, vring_init, we could also reuse the linux implementation
unless we have strong reason to define our own structure.
One reason was to support both FreeBSD and Linux. FreeBSD should have
its own header file. To avoid the case they have different vring
structure or VIRTIO_F_xx macro name, they are redefined here.

>