From: "Xie, Huawei" <huawei.xie@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] virtio: fix used ring address calculation
Date: Thu, 24 Sep 2015 18:35:37 +0000 [thread overview]
Message-ID: <C37D651A908B024F974696C65296B57B40F2E435@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20150924093617.6fd22053@urahara>
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.
>
next prev parent reply other threads:[~2015-09-24 18:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-21 3:39 Huawei Xie
2015-09-21 9:19 ` Tan, Jianfeng
2015-09-24 7:30 ` Xie, Huawei
2015-09-24 16:36 ` Stephen Hemminger
2015-09-24 18:35 ` Xie, Huawei [this message]
2015-09-24 21:01 ` Stephen Hemminger
2015-09-25 15:46 ` Xie, Huawei
2015-09-25 17:48 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=C37D651A908B024F974696C65296B57B40F2E435@SHSMSX101.ccr.corp.intel.com \
--to=huawei.xie@intel.com \
--cc=dev@dpdk.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).