Re: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-25 Thread Wei Xu 
On Fri, Nov 23, 2018 at 01:57:37PM +0800, Wei Xu wrote:
> On Thu, Nov 22, 2018 at 06:57:31PM +0100, Maxime Coquelin wrote:
> > Hi Wei,
> > 
> > I just tested your series with Tiwei's v3, and it fails
> > with ctrl vq enabled:
> > qemu-system-x86_64: virtio-net ctrl missing headers
> 
> OK, I haven't tried Tiwei's v3 yet, will give it a try.

Hi Maxime,
It is caused by the _F_NEXT flag bit for indirect descriptor as
mentioned by tiwei, this patch is needed to fix it.

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 7487d3d..8e61e6f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1478,7 +1478,11 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t 
sz)
 i -= vq->vring.num;
 }
 
-if (desc.flags & VRING_DESC_F_NEXT) {
+if (cache == _desc_cache) {
+if (i == max)
+break;
+vring_packed_desc_read(vq->vdev, , cache, i);
+} else if (desc.flags & VRING_DESC_F_NEXT) {
 vring_packed_desc_read(vq->vdev, , cache, i);
 } else {

Re: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-23 Thread no-reply 
Hi,

This series failed docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Message-id: 1542895581-10721-1-git-send-email-w...@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
d16b38b virtio: enable packed ring via a new command line
e4e3101 vhost: enable packed ring
d9c4fa1 virtio: packed ring feature bit for userspace backend
868a734 virtio: add vhost-net migration of packed ring
0418599 virtio: add userspace migration of packed ring
ff50e22 virtio-net: fill head desc after done all in a chain
bc60f32 virtio: event suppression support for packed ring
e6ed126 virtio: fill/flush/pop for packed ring
782528d virtio: get avail bytes check for packed ring
72bf952 virtio: init and desc empty check for packed ring
c4131b3 virtio: init wrap counter for packed ring
00f26bd virtio: add memory region init for packed ring
279caea virtio: expand offset calculation for packed ring
9dc3712 virtio: redefine structure & memory cache for packed ring
d1a145c virtio: introduce packed ring definitions

=== OUTPUT BEGIN ===
  BUILD   centos7
make[1]: Entering directory `/var/tmp/patchew-tester-tmp-372903yd/src'
  GEN 
/var/tmp/patchew-tester-tmp-372903yd/src/docker-src.2018-11-23-01.39.18.25572/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-372903yd/src/docker-src.2018-11-23-01.39.18.25572/qemu.tar.vroot'...
done.
Checking out files:  48% (3125/6461)   
Checking out files:  49% (3166/6461)   
Checking out files:  50% (3231/6461)   
Checking out files:  51% (3296/6461)   
Checking out files:  52% (3360/6461)   
Checking out files:  53% (3425/6461)   
Checking out files:  54% (3489/6461)   
Checking out files:  55% (3554/6461)   
Checking out files:  56% (3619/6461)   
Checking out files:  57% (3683/6461)   
Checking out files:  58% (3748/6461)   
Checking out files:  59% (3812/6461)   
Checking out files:  60% (3877/6461)   
Checking out files:  61% (3942/6461)   
Checking out files:  62% (4006/6461)   
Checking out files:  63% (4071/6461)   
Checking out files:  64% (4136/6461)   
Checking out files:  65% (4200/6461)   
Checking out files:  66% (4265/6461)   
Checking out files:  67% (4329/6461)   
Checking out files:  68% (4394/6461)   
Checking out files:  69% (4459/6461)   
Checking out files:  70% (4523/6461)   
Checking out files:  71% (4588/6461)   
Checking out files:  72% (4652/6461)   
Checking out files:  73% (4717/6461)   
Checking out files:  74% (4782/6461)   
Checking out files:  75% (4846/6461)   
Checking out files:  76% (4911/6461)   
Checking out files:  77% (4975/6461)   
Checking out files:  78% (5040/6461)   
Checking out files:  79% (5105/6461)   
Checking out files:  80% (5169/6461)   
Checking out files:  81% (5234/6461)   
Checking out files:  82% (5299/6461)   
Checking out files:  83% (5363/6461)   
Checking out files:  84% (5428/6461)   
Checking out files:  85% (5492/6461)   
Checking out files:  86% (5557/6461)   
Checking out files:  87% (5622/6461)   
Checking out files:  88% (5686/6461)   
Checking out files:  89% (5751/6461)   
Checking out files:  90% (5815/6461)   
Checking out files:  91% (5880/6461)   
Checking out files:  92% (5945/6461)   
Checking out files:  93% (6009/6461)   
Checking out files:  94% (6074/6461)   
Checking out files:  95% (6138/6461)   
Checking out files:  96% (6203/6461)   
Checking out files:  97% (6268/6461)   
Checking out files:  98% (6332/6461)   
Checking out files:  99% (6397/6461)   
Checking out files: 100% (6461/6461)   
Checking out files: 100% (6461/6461), done.
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos7 
Packages installed:
SDL-devel-1.2.15-14.el7.x86_64
bison-3.0.4-1.el7.x86_64
bzip2-1.0.6-13.el7.x86_64
bzip2-devel-1.0.6-13.el7.x86_64
ccache-3.3.4-1.el7.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el7.x86_64
flex-2.5.37-3.el7.x86_64
gcc-4.8.5-28.el7_5.1.x86_64
gettext-0.19.8.1-2.el7.x86_64
git-1.8.3.1-14.el7_5.x86_64
glib2-devel-2.54.2-2.el7.x86_64
libaio-devel-0.3.109-13.el7.x86_64
libepoxy-devel-1.3.1-2.el7_5.x86_64
libfdt-devel-1.4.6-1.el7.x86_64
lzo-devel-2.06-8.el7.x86_64
make-3.82-23.el7.x86_64
mesa-libEGL-devel-17.2.3-8.20171019.el7.x86_64
mesa-libgbm-devel-17.2.3-8.20171019.el7.x86_64
nettle-devel-2.7.1-8.el7.x86_64
package g++ is not installed
package librdmacm-d

Re: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-23 Thread no-reply 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 1542895581-10721-1-git-send-email-w...@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20181123063957.9515-1-kra...@redhat.com -> 
patchew/20181123063957.9515-1-kra...@redhat.com
Switched to a new branch 'test'
d16b38b virtio: enable packed ring via a new command line
e4e3101 vhost: enable packed ring
d9c4fa1 virtio: packed ring feature bit for userspace backend
868a734 virtio: add vhost-net migration of packed ring
0418599 virtio: add userspace migration of packed ring
ff50e22 virtio-net: fill head desc after done all in a chain
bc60f32 virtio: event suppression support for packed ring
e6ed126 virtio: fill/flush/pop for packed ring
782528d virtio: get avail bytes check for packed ring
72bf952 virtio: init and desc empty check for packed ring
c4131b3 virtio: init wrap counter for packed ring
00f26bd virtio: add memory region init for packed ring
279caea virtio: expand offset calculation for packed ring
9dc3712 virtio: redefine structure & memory cache for packed ring
d1a145c virtio: introduce packed ring definitions

=== OUTPUT BEGIN ===
Checking PATCH 1/15: virtio: introduce packed ring definitions...
Checking PATCH 2/15: virtio: redefine structure & memory cache for packed 
ring...
Checking PATCH 3/15: virtio: expand offset calculation for packed ring...
Checking PATCH 4/15: virtio: add memory region init for packed ring...
Checking PATCH 5/15: virtio: init wrap counter for packed ring...
Checking PATCH 6/15: virtio: init and desc empty check for packed ring...
Checking PATCH 7/15: virtio: get avail bytes check for packed ring...
Checking PATCH 8/15: virtio: fill/flush/pop for packed ring...
Checking PATCH 9/15: virtio: event suppression support for packed ring...
ERROR: trailing whitespace
#119: FILE: hw/virtio/virtio.c:2208:
+static bool vring_packed_need_event(VirtQueue *vq, bool wrap, $

total: 1 errors, 0 warnings, 151 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/15: virtio-net: fill head desc after done all in a chain...
Checking PATCH 11/15: virtio: add userspace migration of packed ring...
Checking PATCH 12/15: virtio: add vhost-net migration of packed ring...
Checking PATCH 13/15: virtio: packed ring feature bit for userspace backend...
Checking PATCH 14/15: vhost: enable packed ring...
Checking PATCH 15/15: virtio: enable packed ring via a new command line...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-22 Thread Wei Xu 
On Thu, Nov 22, 2018 at 06:57:31PM +0100, Maxime Coquelin wrote:
> Hi Wei,
> 
> I just tested your series with Tiwei's v3, and it fails
> with ctrl vq enabled:
> qemu-system-x86_64: virtio-net ctrl missing headers

OK, I haven't tried Tiwei's v3 yet, will give it a try.

Wei

> 
> Regards,
> Maxime
> 
> On 11/22/18 3:06 PM, w...@redhat.com wrote:
> >From: Wei Xu 
> >
> >Code base:
> > https://github.com/Whishay/qemu.git
> >
> >rfc v3 -> v1
> >- migration support for both userspace and vhost-net, need tweak vhost
> >   ioctl() to make it work(the code is pasted in the commit message of
> >   vhost migration patch #13).
> >
> >Note:
> >   the high 32-bit guest feature bit is saved as a subsection for
> >   virtio devices which makes packed ring feature bit check unusable when
> >   loading the saved per-queue variables(this is done before loading
> >   subsection which is the last action for device during migration),
> >   so I save and load all the things generally for now, any idea to fix this?
> >
> >- Fixed comments from Jason for rfc v3 sorted by patch #, two comments I
> >   didn't take were(from patch) listed here:
> >09: - introduce new API(virtqueue_fill_n()).
> >   - Didn't take it since userspace backend does not support batching,
> > so only one element is popped and current API should be enough.
> >06 & 07: Refactor split and packed pop()/get_avail_bytes().
> >  - the duplicated code interwined with split/packed ring specific
> >things and it might make it unclear, so I only extracted the few
> >common parts out side rcu and keep the others separate.
> >
> >The other revised comments:
> >02: - reuse current 'avail/used' for 'driver/device' in 
> >VRingMemoryRegionCache.
> > - remove event_idx since shadow_avail_idx works.
> >03: - move size recalculation to a separate patch.
> > - keep 'avail/used' in current calculation function name.
> > - initialize 'desc' memory region as 'false' for 1.0('true' for 1.1)
> >04: - delete 'event_idx'
> >05: - rename 'wc' to wrap_counter.
> >06: - converge common part outside rcu section for 1.0/1.1.
> > - move memory barrier for the first 'desc' in between checking flag
> >   and read other fields.
> > - remove unnecessary memory barriers for indirect descriptors.
> > - no need to destroy indirect memory cache since it is generally done
> >   before return from the function.
> > - remove redundant maximum chained descriptors limitation check.
> > - there are some differences(desc name, wrap idx/counter, flags) between
> >   split and packed rings, so keep them separate for now.
> > - amend the comment when recording index and wrap counter for a kick
> >   from guest.
> >07: - calculate fields in descriptor instead of read it when filling.
> > - put memory barrier correctly before filling the flags in descriptor.
> > - replace full memory barrier with a write barrier in fill.
> > - shift to read descriptor flags and descriptor necessarily and
> >   separately in packed_pop().
> > - correct memory barrier in packed_pop() as in packed_fill().
> >08: - reuse 'shadow_avail_idx' instead of adding a new 'event_idx'.
> > - use the compact and verified vring_packed_need_event()
> >   version for vhost net/user.
> >12: - remove the odd cherry-pick comment.
> > - used bit '15' for wrap_counters.
> >
> >rfc v2->v3
> >- addressed performance issue
> >- fixed feedback from v2
> >
> >rfc v1->v2
> >- sync to tiwei's v5
> >- reuse memory cache function with 1.0
> >- dropped detach patch and notification helper(04 & 05 in v1)
> >- guest virtio-net driver unload/reload support
> >- event suppression support(not tested)
> >- addressed feedback from v1
> >
> >Wei Xu (15):
> >   virtio: introduce packed ring definitions
> >   virtio: redefine structure & memory cache for packed ring
> >   virtio: expand offset calculation for packed ring
> >   virtio: add memory region init for packed ring
> >   virtio: init wrap counter for packed ring
> >   virtio: init and desc empty check for packed ring
> >   virtio: get avail bytes check for packed ring
> >   virtio: fill/flush/pop for packed ring
> >   virtio: event suppression support for packed ring
> >   virtio-net: fill head desc after done all in a chain
> >   virtio: add userspace migration of packed ring
> >   virtio: add vhost-net migration of packed ring
> >   virtio: packed ring feature bit for userspace backend
> >   vhost: enable packed ring
> >   virtio: enable packed ring via a new command line
> >
> >  VERSION|   2 +-
> >  hw/net/vhost_net.c |   2 +
> >  hw/net/virtio-net.c|  11 +-
> >  hw/virtio/virtio.c | 756 
> > +++--
> >  include/hw/virtio/virtio.h |   8 +-
> >  include/standard-headers/linux/virtio_config.h |  15 +
> >

Re: [Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-22 Thread Maxime Coquelin 

Hi Wei,

I just tested your series with Tiwei's v3, and it fails
with ctrl vq enabled:
qemu-system-x86_64: virtio-net ctrl missing headers

Regards,
Maxime

On 11/22/18 3:06 PM, w...@redhat.com wrote:

From: Wei Xu 

Code base:
 https://github.com/Whishay/qemu.git

rfc v3 -> v1
- migration support for both userspace and vhost-net, need tweak vhost
   ioctl() to make it work(the code is pasted in the commit message of
   vhost migration patch #13).

Note:
   the high 32-bit guest feature bit is saved as a subsection for
   virtio devices which makes packed ring feature bit check unusable when
   loading the saved per-queue variables(this is done before loading
   subsection which is the last action for device during migration),
   so I save and load all the things generally for now, any idea to fix this?

- Fixed comments from Jason for rfc v3 sorted by patch #, two comments I
   didn't take were(from patch) listed here:
09: - introduce new API(virtqueue_fill_n()).
   - Didn't take it since userspace backend does not support batching,
 so only one element is popped and current API should be enough.
06 & 07: Refactor split and packed pop()/get_avail_bytes().
  - the duplicated code interwined with split/packed ring specific
things and it might make it unclear, so I only extracted the few
common parts out side rcu and keep the others separate.

The other revised comments:
02: - reuse current 'avail/used' for 'driver/device' in VRingMemoryRegionCache.
 - remove event_idx since shadow_avail_idx works.
03: - move size recalculation to a separate patch.
 - keep 'avail/used' in current calculation function name.
 - initialize 'desc' memory region as 'false' for 1.0('true' for 1.1)
04: - delete 'event_idx'
05: - rename 'wc' to wrap_counter.
06: - converge common part outside rcu section for 1.0/1.1.
 - move memory barrier for the first 'desc' in between checking flag
   and read other fields.
 - remove unnecessary memory barriers for indirect descriptors.
 - no need to destroy indirect memory cache since it is generally done
   before return from the function.
 - remove redundant maximum chained descriptors limitation check.
 - there are some differences(desc name, wrap idx/counter, flags) between
   split and packed rings, so keep them separate for now.
 - amend the comment when recording index and wrap counter for a kick
   from guest.
07: - calculate fields in descriptor instead of read it when filling.
 - put memory barrier correctly before filling the flags in descriptor.
 - replace full memory barrier with a write barrier in fill.
 - shift to read descriptor flags and descriptor necessarily and
   separately in packed_pop().
 - correct memory barrier in packed_pop() as in packed_fill().
08: - reuse 'shadow_avail_idx' instead of adding a new 'event_idx'.
 - use the compact and verified vring_packed_need_event()
   version for vhost net/user.
12: - remove the odd cherry-pick comment.
 - used bit '15' for wrap_counters.

rfc v2->v3
- addressed performance issue
- fixed feedback from v2

rfc v1->v2
- sync to tiwei's v5
- reuse memory cache function with 1.0
- dropped detach patch and notification helper(04 & 05 in v1)
- guest virtio-net driver unload/reload support
- event suppression support(not tested)
- addressed feedback from v1

Wei Xu (15):
   virtio: introduce packed ring definitions
   virtio: redefine structure & memory cache for packed ring
   virtio: expand offset calculation for packed ring
   virtio: add memory region init for packed ring
   virtio: init wrap counter for packed ring
   virtio: init and desc empty check for packed ring
   virtio: get avail bytes check for packed ring
   virtio: fill/flush/pop for packed ring
   virtio: event suppression support for packed ring
   virtio-net: fill head desc after done all in a chain
   virtio: add userspace migration of packed ring
   virtio: add vhost-net migration of packed ring
   virtio: packed ring feature bit for userspace backend
   vhost: enable packed ring
   virtio: enable packed ring via a new command line

  VERSION|   2 +-
  hw/net/vhost_net.c |   2 +
  hw/net/virtio-net.c|  11 +-
  hw/virtio/virtio.c | 756 +++--
  include/hw/virtio/virtio.h |   8 +-
  include/standard-headers/linux/virtio_config.h |  15 +
  include/standard-headers/linux/virtio_ring.h   |  43 ++
  7 files changed, 783 insertions(+), 54 deletions(-)

[Qemu-devel] [PATCH v1 00/16] packed ring virtio-net backend support

2018-11-22 Thread wexu 
From: Wei Xu 

Code base:
https://github.com/Whishay/qemu.git

rfc v3 -> v1
- migration support for both userspace and vhost-net, need tweak vhost
  ioctl() to make it work(the code is pasted in the commit message of
  vhost migration patch #13).

Note: 
  the high 32-bit guest feature bit is saved as a subsection for
  virtio devices which makes packed ring feature bit check unusable when
  loading the saved per-queue variables(this is done before loading 
  subsection which is the last action for device during migration),
  so I save and load all the things generally for now, any idea to fix this?

- Fixed comments from Jason for rfc v3 sorted by patch #, two comments I
  didn't take were(from patch) listed here:
09: - introduce new API(virtqueue_fill_n()).
  - Didn't take it since userspace backend does not support batching,
so only one element is popped and current API should be enough.
06 & 07: Refactor split and packed pop()/get_avail_bytes().
 - the duplicated code interwined with split/packed ring specific
   things and it might make it unclear, so I only extracted the few
   common parts out side rcu and keep the others separate.

The other revised comments:
02: - reuse current 'avail/used' for 'driver/device' in VRingMemoryRegionCache.
- remove event_idx since shadow_avail_idx works.
03: - move size recalculation to a separate patch.
- keep 'avail/used' in current calculation function name.
- initialize 'desc' memory region as 'false' for 1.0('true' for 1.1)
04: - delete 'event_idx'
05: - rename 'wc' to wrap_counter.
06: - converge common part outside rcu section for 1.0/1.1.
- move memory barrier for the first 'desc' in between checking flag
  and read other fields.
- remove unnecessary memory barriers for indirect descriptors.
- no need to destroy indirect memory cache since it is generally done
  before return from the function.
- remove redundant maximum chained descriptors limitation check.
- there are some differences(desc name, wrap idx/counter, flags) between
  split and packed rings, so keep them separate for now.
- amend the comment when recording index and wrap counter for a kick
  from guest.
07: - calculate fields in descriptor instead of read it when filling.
- put memory barrier correctly before filling the flags in descriptor.
- replace full memory barrier with a write barrier in fill.
- shift to read descriptor flags and descriptor necessarily and
  separately in packed_pop().
- correct memory barrier in packed_pop() as in packed_fill().
08: - reuse 'shadow_avail_idx' instead of adding a new 'event_idx'.
- use the compact and verified vring_packed_need_event()
  version for vhost net/user.
12: - remove the odd cherry-pick comment.
- used bit '15' for wrap_counters.

rfc v2->v3
- addressed performance issue
- fixed feedback from v2

rfc v1->v2
- sync to tiwei's v5
- reuse memory cache function with 1.0
- dropped detach patch and notification helper(04 & 05 in v1)
- guest virtio-net driver unload/reload support
- event suppression support(not tested)
- addressed feedback from v1

Wei Xu (15):
  virtio: introduce packed ring definitions
  virtio: redefine structure & memory cache for packed ring
  virtio: expand offset calculation for packed ring
  virtio: add memory region init for packed ring
  virtio: init wrap counter for packed ring
  virtio: init and desc empty check for packed ring
  virtio: get avail bytes check for packed ring
  virtio: fill/flush/pop for packed ring
  virtio: event suppression support for packed ring
  virtio-net: fill head desc after done all in a chain
  virtio: add userspace migration of packed ring
  virtio: add vhost-net migration of packed ring
  virtio: packed ring feature bit for userspace backend
  vhost: enable packed ring
  virtio: enable packed ring via a new command line

 VERSION|   2 +-
 hw/net/vhost_net.c |   2 +
 hw/net/virtio-net.c|  11 +-
 hw/virtio/virtio.c | 756 +++--
 include/hw/virtio/virtio.h |   8 +-
 include/standard-headers/linux/virtio_config.h |  15 +
 include/standard-headers/linux/virtio_ring.h   |  43 ++
 7 files changed, 783 insertions(+), 54 deletions(-)

-- 
1.8.3.1