* [dpdk-dev] [PATCH] vhost: fix notification for packed ring
@ 2018-10-11 13:06 Tiwei Bie
2018-10-11 13:17 ` Maxime Coquelin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Tiwei Bie @ 2018-10-11 13:06 UTC (permalink / raw)
To: maxime.coquelin, zhihong.wang, dev; +Cc: mst, jasowang, stable
The notification can't be disabled in packed ring when
application tries to disable notification, because the
device event flags field is overwritten by an unexpected
value. This patch fixes this issue.
Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
Cc: stable@dpdk.org
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
lib/librte_vhost/vhost.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e62f4c594..c9270bdec 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
{
uint16_t flags;
- if (!enable)
- vq->device_event->flags = VRING_EVENT_F_DISABLE;
+ if (!enable) {
+ flags = VRING_EVENT_F_DISABLE;
+ goto out;
+ }
flags = VRING_EVENT_F_ENABLE;
if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
@@ -677,6 +679,7 @@ vhost_enable_notify_packed(struct virtio_net *dev,
vq->avail_wrap_counter << 15;
}
+out:
rte_smp_wmb();
vq->device_event->flags = flags;
--
2.19.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix notification for packed ring
2018-10-11 13:06 [dpdk-dev] [PATCH] vhost: fix notification for packed ring Tiwei Bie
@ 2018-10-11 13:17 ` Maxime Coquelin
2018-10-11 13:34 ` Jason Wang
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
2 siblings, 0 replies; 9+ messages in thread
From: Maxime Coquelin @ 2018-10-11 13:17 UTC (permalink / raw)
To: Tiwei Bie, zhihong.wang, dev; +Cc: mst, jasowang, stable
On 10/11/2018 03:06 PM, Tiwei Bie wrote:
> The notification can't be disabled in packed ring when
> application tries to disable notification, because the
> device event flags field is overwritten by an unexpected
> value. This patch fixes this issue.
>
> Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> lib/librte_vhost/vhost.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index e62f4c594..c9270bdec 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> {
> uint16_t flags;
>
> - if (!enable)
> - vq->device_event->flags = VRING_EVENT_F_DISABLE;
> + if (!enable) {
> + flags = VRING_EVENT_F_DISABLE;
> + goto out;
> + }
>
> flags = VRING_EVENT_F_ENABLE;
> if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
> @@ -677,6 +679,7 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> vq->avail_wrap_counter << 15;
> }
>
> +out:
> rte_smp_wmb();
>
> vq->device_event->flags = flags;
>
Thanks for spotting this:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Maxime
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix notification for packed ring
2018-10-11 13:06 [dpdk-dev] [PATCH] vhost: fix notification for packed ring Tiwei Bie
2018-10-11 13:17 ` Maxime Coquelin
@ 2018-10-11 13:34 ` Jason Wang
2018-10-11 13:36 ` Michael S. Tsirkin
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
2 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2018-10-11 13:34 UTC (permalink / raw)
To: Tiwei Bie, maxime.coquelin, zhihong.wang, dev; +Cc: mst, stable
On 2018年10月11日 21:06, Tiwei Bie wrote:
> The notification can't be disabled in packed ring when
> application tries to disable notification, because the
> device event flags field is overwritten by an unexpected
> value. This patch fixes this issue.
>
> Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> lib/librte_vhost/vhost.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index e62f4c594..c9270bdec 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> {
> uint16_t flags;
>
> - if (!enable)
> - vq->device_event->flags = VRING_EVENT_F_DISABLE;
> + if (!enable) {
> + flags = VRING_EVENT_F_DISABLE;
> + goto out;
> + }
>
> flags = VRING_EVENT_F_ENABLE;
> if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
> @@ -677,6 +679,7 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> vq->avail_wrap_counter << 15;
> }
>
> +out:
> rte_smp_wmb();
>
It looks to me the barrier is even not needed for !enable.
Thanks
> vq->device_event->flags = flags;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix notification for packed ring
2018-10-11 13:34 ` Jason Wang
@ 2018-10-11 13:36 ` Michael S. Tsirkin
2018-10-11 14:10 ` Tiwei Bie
0 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2018-10-11 13:36 UTC (permalink / raw)
To: Jason Wang; +Cc: Tiwei Bie, maxime.coquelin, zhihong.wang, dev, stable
On Thu, Oct 11, 2018 at 09:34:06PM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 21:06, Tiwei Bie wrote:
> > The notification can't be disabled in packed ring when
> > application tries to disable notification, because the
> > device event flags field is overwritten by an unexpected
> > value. This patch fixes this issue.
> >
> > Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > lib/librte_vhost/vhost.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> > index e62f4c594..c9270bdec 100644
> > --- a/lib/librte_vhost/vhost.c
> > +++ b/lib/librte_vhost/vhost.c
> > @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> > {
> > uint16_t flags;
> > - if (!enable)
> > - vq->device_event->flags = VRING_EVENT_F_DISABLE;
> > + if (!enable) {
> > + flags = VRING_EVENT_F_DISABLE;
> > + goto out;
> > + }
> > flags = VRING_EVENT_F_ENABLE;
> > if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
> > @@ -677,6 +679,7 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> > vq->avail_wrap_counter << 15;
> > }
> > +out:
> > rte_smp_wmb();
>
> It looks to me the barrier is even not needed for !enable.
>
> Thanks
Good point, I agree.
Besides that
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > vq->device_event->flags = flags;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix notification for packed ring
2018-10-11 13:36 ` Michael S. Tsirkin
@ 2018-10-11 14:10 ` Tiwei Bie
0 siblings, 0 replies; 9+ messages in thread
From: Tiwei Bie @ 2018-10-11 14:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jason Wang, maxime.coquelin, zhihong.wang, dev, stable
On Thu, Oct 11, 2018 at 09:36:48AM -0400, Michael S. Tsirkin wrote:
> On Thu, Oct 11, 2018 at 09:34:06PM +0800, Jason Wang wrote:
> >
> >
> > On 2018年10月11日 21:06, Tiwei Bie wrote:
> > > The notification can't be disabled in packed ring when
> > > application tries to disable notification, because the
> > > device event flags field is overwritten by an unexpected
> > > value. This patch fixes this issue.
> > >
> > > Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > ---
> > > lib/librte_vhost/vhost.c | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> > > index e62f4c594..c9270bdec 100644
> > > --- a/lib/librte_vhost/vhost.c
> > > +++ b/lib/librte_vhost/vhost.c
> > > @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> > > {
> > > uint16_t flags;
> > > - if (!enable)
> > > - vq->device_event->flags = VRING_EVENT_F_DISABLE;
> > > + if (!enable) {
> > > + flags = VRING_EVENT_F_DISABLE;
> > > + goto out;
> > > + }
> > > flags = VRING_EVENT_F_ENABLE;
> > > if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
> > > @@ -677,6 +679,7 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> > > vq->avail_wrap_counter << 15;
> > > }
> > > +out:
> > > rte_smp_wmb();
> >
> > It looks to me the barrier is even not needed for !enable.
> >
> > Thanks
>
> Good point, I agree.
> Besides that
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
Yeah, it's better to return directly after disabling the
interrupt. I will send a new version to do this change.
Thanks!
>
>
> > > vq->device_event->flags = flags;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2] vhost: fix notification for packed ring
2018-10-11 13:06 [dpdk-dev] [PATCH] vhost: fix notification for packed ring Tiwei Bie
2018-10-11 13:17 ` Maxime Coquelin
2018-10-11 13:34 ` Jason Wang
@ 2018-10-11 14:22 ` Tiwei Bie
2018-10-11 15:21 ` Maxime Coquelin
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: Tiwei Bie @ 2018-10-11 14:22 UTC (permalink / raw)
To: maxime.coquelin, zhihong.wang, dev; +Cc: mst, jasowang, stable
The notification can't be disabled in packed ring when
application tries to disable notification, because the
device event flags field is overwritten by an unexpected
value. This patch fixes this issue.
Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
Cc: stable@dpdk.org
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
v2: return directly after disabling the interrupt (Jason/MST)
lib/librte_vhost/vhost.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index e62f4c594..047ee535c 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
{
uint16_t flags;
- if (!enable)
+ if (!enable) {
vq->device_event->flags = VRING_EVENT_F_DISABLE;
+ return;
+ }
flags = VRING_EVENT_F_ENABLE;
if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
--
2.19.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix notification for packed ring
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
@ 2018-10-11 15:21 ` Maxime Coquelin
2018-10-11 23:47 ` Jason Wang
2018-10-16 8:36 ` Maxime Coquelin
2 siblings, 0 replies; 9+ messages in thread
From: Maxime Coquelin @ 2018-10-11 15:21 UTC (permalink / raw)
To: Tiwei Bie, zhihong.wang, dev; +Cc: mst, jasowang, stable
On 10/11/2018 04:22 PM, Tiwei Bie wrote:
> The notification can't be disabled in packed ring when
> application tries to disable notification, because the
> device event flags field is overwritten by an unexpected
> value. This patch fixes this issue.
>
> Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> v2: return directly after disabling the interrupt (Jason/MST)
>
> lib/librte_vhost/vhost.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index e62f4c594..047ee535c 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> {
> uint16_t flags;
>
> - if (!enable)
> + if (!enable) {
> vq->device_event->flags = VRING_EVENT_F_DISABLE;
> + return;
> + }
>
> flags = VRING_EVENT_F_ENABLE;
> if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix notification for packed ring
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
2018-10-11 15:21 ` Maxime Coquelin
@ 2018-10-11 23:47 ` Jason Wang
2018-10-16 8:36 ` Maxime Coquelin
2 siblings, 0 replies; 9+ messages in thread
From: Jason Wang @ 2018-10-11 23:47 UTC (permalink / raw)
To: Tiwei Bie, maxime.coquelin, zhihong.wang, dev; +Cc: mst, stable
On 2018年10月11日 22:22, Tiwei Bie wrote:
> The notification can't be disabled in packed ring when
> application tries to disable notification, because the
> device event flags field is overwritten by an unexpected
> value. This patch fixes this issue.
>
> Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v2: return directly after disabling the interrupt (Jason/MST)
>
> lib/librte_vhost/vhost.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index e62f4c594..047ee535c 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -667,8 +667,10 @@ vhost_enable_notify_packed(struct virtio_net *dev,
> {
> uint16_t flags;
>
> - if (!enable)
> + if (!enable) {
> vq->device_event->flags = VRING_EVENT_F_DISABLE;
> + return;
> + }
>
> flags = VRING_EVENT_F_ENABLE;
> if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX)) {
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: fix notification for packed ring
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
2018-10-11 15:21 ` Maxime Coquelin
2018-10-11 23:47 ` Jason Wang
@ 2018-10-16 8:36 ` Maxime Coquelin
2 siblings, 0 replies; 9+ messages in thread
From: Maxime Coquelin @ 2018-10-16 8:36 UTC (permalink / raw)
To: Tiwei Bie, zhihong.wang, dev; +Cc: mst, jasowang, stable
On 10/11/2018 04:22 PM, Tiwei Bie wrote:
> The notification can't be disabled in packed ring when
> application tries to disable notification, because the
> device event flags field is overwritten by an unexpected
> value. This patch fixes this issue.
>
> Fixes: b1cce26af1dc ("vhost: add notification for packed ring")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v2: return directly after disabling the interrupt (Jason/MST)
>
> lib/librte_vhost/vhost.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Applied to dpdk-next-virtio/master.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-16 8:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 13:06 [dpdk-dev] [PATCH] vhost: fix notification for packed ring Tiwei Bie
2018-10-11 13:17 ` Maxime Coquelin
2018-10-11 13:34 ` Jason Wang
2018-10-11 13:36 ` Michael S. Tsirkin
2018-10-11 14:10 ` Tiwei Bie
2018-10-11 14:22 ` [dpdk-dev] [PATCH v2] " Tiwei Bie
2018-10-11 15:21 ` Maxime Coquelin
2018-10-11 23:47 ` Jason Wang
2018-10-16 8:36 ` Maxime Coquelin
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).