[Qemu-devel] Networking patches queue

[Qemu-devel] Networking patches queue

[Mark McLoughlin]

Hi Anthony,

Recently, Jan has posted 11 networking patches and I've posted 17, so I
thought I'd push out a tree with these queued up. Perhaps you want to
pull from there?

Some notes:

   - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
     the review comments I just posted. I expect Jan will be able to 
     fix them up fairly quickly

   - I've tried my best to fix up the param checking saga by reverting 
     Kevin's patch, going with Jan's rollback to something closer to 
     what was there originally and applying a small fixup patch

   - Not all of these patches are completely isolated to networking 
     code - e.g. the fork_exec() patch adds a SIGCHLD handler

   - I haven't reviewed the slirp changes in great detail, but they 
     look okay at a glance

The output of git pull-request:

The following changes since commit abc0754527e30acf278765f66d2157b6c75dc549:
  Edgar E. Iglesias (1):
        Update maintainer list.

are available in the git repository at:

  git://git.et.redhat.com/qemu-net.git queue

Jan Kiszka (6):
      net: Don't deliver to disabled interfaces in qemu_sendv_packet
      net: Fix and improved ordered packet delivery
      slirp: Avoid zombie processes after fork_exec
      net: Real fix for check_params users
      net: Improve parameter error reporting
      slirp: Reorder initialization

Mark McLoughlin (15):
      Revert "Fix output of uninitialized strings"
      net: fix error reporting for some net parameter checks
      net: factor tap_read_packet() out of tap_send()
      net: move the tap buffer into TAPState
      net: vlan clients with no fd_can_read() can always receive
      net: only read from tapfd when we can send
      net: add fd_readv() handler to qemu_new_vlan_client() args
      net: re-name vc->fd_read() to vc->receive()
      net: pass VLANClientState* as first arg to receive handlers
      net: add return value to packet receive handler
      net: return status from qemu_deliver_packet()
      net: split out packet queueing and flushing into separate functions
      net: add qemu_send_packet_async()
      net: make use of async packet sending API in tap client
      virtio-net: implement rx packet queueing

 hw/dp8393x.c        |   22 +-
 hw/e1000.c          |   30 ++-
 hw/eepro100.c       |   23 +-
 hw/etraxfs_eth.c    |   14 +-
 hw/mcf_fec.c        |   11 +-
 hw/mipsnet.c        |   16 +-
 hw/musicpal.c       |   11 +-
 hw/ne2000.c         |   25 +-
 hw/pci-hotplug.c    |    7 +-
 hw/pcnet.c          |   17 +-
 hw/qdev.c           |    9 +-
 hw/rtl8139.c        |   39 ++--
 hw/smc91c111.c      |   18 +-
 hw/stellaris_enet.c |   20 +-
 hw/usb-net.c        |   18 +-
 hw/virtio-net.c     |   21 +-
 hw/xen_nic.c        |   26 +-
 net.c               |  708 ++++++++++++++++++++++++++++++++++-----------------
 net.h               |   31 ++-
 savevm.c            |    2 +-
 slirp/libslirp.h    |    2 +-
 slirp/slirp.c       |    2 +-
 sysemu.h            |    3 +-
 tap-win32.c         |    8 +-
 vl.c                |   57 ++--
 25 files changed, 716 insertions(+), 424 deletions(-)

Cheers,
Mark.

[Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Mark McLoughlin wrote:
> Hi Anthony,
>
> Recently, Jan has posted 11 networking patches and I've posted 17, so I
> thought I'd push out a tree with these queued up. Perhaps you want to
> pull from there?
>
> Some notes:
>
>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>      the review comments I just posted. I expect Jan will be able to 
>      fix them up fairly quickly
>   

If the first 6 patches of Jan's series are ready to apply, wouldn't it
make sense for him to submit that as a separate series?  In the very
least, I'd like an Ack from Jan before applying his series partially.

>    - I've tried my best to fix up the param checking saga by reverting 
>      Kevin's patch, going with Jan's rollback to something closer to 
>      what was there originally and applying a small fixup patch
>
>    - Not all of these patches are completely isolated to networking 
>      code - e.g. the fork_exec() patch adds a SIGCHLD handler
>
>    - I haven't reviewed the slirp changes in great detail, but they 
>      look okay at a glance
>   

I just got the tail end of your series before heading off on travel on
Friday.  It still needs review and testing.

Of course, if a patches series included test cases for the functionality
it was implementing, it would certainly go a far way into reducing the
amount of time it took to test those patches :-)

Regards,

Anthony Liguori

[Qemu-devel] Re: Networking patches queue

[Jan Kiszka]

Anthony Liguori wrote:
> Mark McLoughlin wrote:
>> Hi Anthony,
>>
>> Recently, Jan has posted 11 networking patches and I've posted 17, so I
>> thought I'd push out a tree with these queued up. Perhaps you want to
>> pull from there?
>>
>> Some notes:
>>
>>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>>      the review comments I just posted. I expect Jan will be able to 
>>      fix them up fairly quickly
>>   
> 
> If the first 6 patches of Jan's series are ready to apply, wouldn't it
> make sense for him to submit that as a separate series?  In the very
> least, I'd like an Ack from Jan before applying his series partially.

You have the ack now. I'm very happy that Mark picks this up before it
started to bitrot too much.

Mark, do you plan more work in this domain in the next time? I would
have no problems to route my networking related stuff through one
coordinating tree, e.g. yours. Besides getting my current queue flushed
I still have

 o rework of host_net_redir (requires coordination with Glauber)
 o multi-instance slirp

on my agenda. And maybe more will comes once this is rolled out.

> 
>>    - I've tried my best to fix up the param checking saga by reverting 
>>      Kevin's patch, going with Jan's rollback to something closer to 
>>      what was there originally and applying a small fixup patch
>>
>>    - Not all of these patches are completely isolated to networking 
>>      code - e.g. the fork_exec() patch adds a SIGCHLD handler
>>
>>    - I haven't reviewed the slirp changes in great detail, but they 
>>      look okay at a glance
>>   
> 
> I just got the tail end of your series before heading off on travel on
> Friday.  It still needs review and testing.
> 
> Of course, if a patches series included test cases for the functionality
> it was implementing, it would certainly go a far way into reducing the
> amount of time it took to test those patches :-)

Well, with a test framework for qemu upstream...

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: Networking patches queue

[Mark McLoughlin]

On Thu, 2009-05-28 at 10:28 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > Hi Anthony,
> >
> > Recently, Jan has posted 11 networking patches and I've posted 17, so I
> > thought I'd push out a tree with these queued up. Perhaps you want to
> > pull from there?
> >
> > Some notes:
> >
> >    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
> >      the review comments I just posted. I expect Jan will be able to 
> >      fix them up fairly quickly
> >   
> 
> If the first 6 patches of Jan's series are ready to apply, wouldn't it
> make sense for him to submit that as a separate series?  In the very
> least, I'd like an Ack from Jan before applying his series partially.

I would have thought that was one of the benefits of a clean, bisectable
patch series - that a maintainer could choose to partially apply them.

I know when I send a patch series, I'm fully prepared for that to happen
and would much rather see them applied partially if possible, rather
than re-send the whole series.

(Where partially means 1-N not randomly choosing a subset of patches)

> >    - I've tried my best to fix up the param checking saga by reverting 
> >      Kevin's patch, going with Jan's rollback to something closer to 
> >      what was there originally and applying a small fixup patch
> >
> >    - Not all of these patches are completely isolated to networking 
> >      code - e.g. the fork_exec() patch adds a SIGCHLD handler
> >
> >    - I haven't reviewed the slirp changes in great detail, but they 
> >      look okay at a glance
> >   
> 
> I just got the tail end of your series before heading off on travel on
> Friday.  It still needs review and testing.

Okay, that's perfectly reasonable.

I got the impression you would like (at some point) to be able to have
others to act as a funnel for specific areas. I'm just testing the
water :-)

> Of course, if a patches series included test cases for the functionality
> it was implementing, it would certainly go a far way into reducing the
> amount of time it took to test those patches :-)

That's a funny way of observing "we really should have a networking test
suite".

Would "this tree passes kvm-autotest's networking tests" help matters?
If so, I'm sure that could be organised ...

Cheers,
Mark.

[Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Mark McLoughlin wrote:
> On Thu, 2009-05-28 at 10:28 -0500, Anthony Liguori wrote:
>   
>> If the first 6 patches of Jan's series are ready to apply, wouldn't it
>> make sense for him to submit that as a separate series?  In the very
>> least, I'd like an Ack from Jan before applying his series partially.
>>     
>
> I would have thought that was one of the benefits of a clean, bisectable
> patch series - that a maintainer could choose to partially apply them.
>
> I know when I send a patch series, I'm fully prepared for that to happen
> and would much rather see them applied partially if possible, rather
> than re-send the whole series.
>
> (Where partially means 1-N not randomly choosing a subset of patches)
>   

I used to do that, but not everyone likes it.

>> I just got the tail end of your series before heading off on travel on
>> Friday.  It still needs review and testing.
>>     
>
> Okay, that's perfectly reasonable.
>
> I got the impression you would like (at some point) to be able to have
> others to act as a funnel for specific areas. I'm just testing the
> water :-)
>   

Yes, that's what I was hinting at below :-)

>> Of course, if a patches series included test cases for the functionality
>> it was implementing, it would certainly go a far way into reducing the
>> amount of time it took to test those patches :-)
>>     
>
> That's a funny way of observing "we really should have a networking test
> suite".
>
> Would "this tree passes kvm-autotest's networking tests" help matters?
> If so, I'm sure that could be organised ...
>   

Does kvm-autotest have networking tests?

I know there's some concern about the time it takes for patches to get
applied.  IMHO, the best way to improve that it to get a stronger set of
functional tests.  I have a set I've been working on but it's on my
other laptop which I cannot reach ATM :-/

As has been discussed before, I'm looking for finer grain functional
tests than the installation tests that kvm-autotest is providing now. 
I'm not saying that this series requires submitting a test suite first,
but rather that if you're interested in seeing networking advance more
rapidly, a good investment would be to build out a better test
infrastructure for it.

Regards,

Anthony Liguori

[Qemu-devel] Re: Networking patches queue

[Mark McLoughlin]

On Thu, 2009-05-28 at 10:56 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > On Thu, 2009-05-28 at 10:28 -0500, Anthony Liguori wrote:
> >   
> >> If the first 6 patches of Jan's series are ready to apply, wouldn't it
> >> make sense for him to submit that as a separate series?  In the very
> >> least, I'd like an Ack from Jan before applying his series partially.
> >>     
> >
> > I would have thought that was one of the benefits of a clean, bisectable
> > patch series - that a maintainer could choose to partially apply them.
> >
> > I know when I send a patch series, I'm fully prepared for that to happen
> > and would much rather see them applied partially if possible, rather
> > than re-send the whole series.
> >
> > (Where partially means 1-N not randomly choosing a subset of patches)
> >   
> 
> I used to do that, but not everyone likes it.
> 
> >> I just got the tail end of your series before heading off on travel on
> >> Friday.  It still needs review and testing.
> >>     
> >
> > Okay, that's perfectly reasonable.
> >
> > I got the impression you would like (at some point) to be able to have
> > others to act as a funnel for specific areas. I'm just testing the
> > water :-)
> >   
> 
> Yes, that's what I was hinting at below :-)
> 
> >> Of course, if a patches series included test cases for the functionality
> >> it was implementing, it would certainly go a far way into reducing the
> >> amount of time it took to test those patches :-)
> >>     
> >
> > That's a funny way of observing "we really should have a networking test
> > suite".
> >
> > Would "this tree passes kvm-autotest's networking tests" help matters?
> > If so, I'm sure that could be organised ...
> >   
> 
> Does kvm-autotest have networking tests?

No targeted networking tests yet, no.

> I know there's some concern about the time it takes for patches to get
> applied.  IMHO, the best way to improve that it to get a stronger set of
> functional tests.  I have a set I've been working on but it's on my
> other laptop which I cannot reach ATM :-/

Okay, very interested in seeing that.

> As has been discussed before, I'm looking for finer grain functional
> tests than the installation tests that kvm-autotest is providing now. 
> I'm not saying that this series requires submitting a test suite first,
> but rather that if you're interested in seeing networking advance more
> rapidly, a good investment would be to build out a better test
> infrastructure for it.

I missed the autotest discussion earlier in the week, just saw it now.


Cheers,
Mark.

[Qemu-devel] Re: Networking patches queue

[Mark McLoughlin]

On Thu, 2009-05-28 at 17:51 +0200, Jan Kiszka wrote:

> Mark, do you plan more work in this domain in the next time?

Time permitting, I'd like to re-start the work I was doing to make a
(nic, host backend) pair our default networking mode and to de-emphasise
the VLAN concept. With that done we could merge the IFF_VNET_HDR support
from qemu-kvm.git.

>  I would
> have no problems to route my networking related stuff through one
> coordinating tree, e.g. yours. Besides getting my current queue flushed
> I still have
> 
>  o rework of host_net_redir (requires coordination with Glauber)
>  o multi-instance slirp
> 
> on my agenda. And maybe more will comes once this is rolled out.

Cool. We'll see how it goes. Unless Anthony is actually pulling from
this tree, it's only useful when we have a bunch of inter-dependant
queued patches.

Cheers,
Mark.

[Qemu-devel] Re: Networking patches queue

[Glauber Costa]

On Thu, May 28, 2009 at 05:51:09PM +0200, Jan Kiszka wrote:
> Anthony Liguori wrote:
> > Mark McLoughlin wrote:
> >> Hi Anthony,
> >>
> >> Recently, Jan has posted 11 networking patches and I've posted 17, so I
> >> thought I'd push out a tree with these queued up. Perhaps you want to
> >> pull from there?
> >>
> >> Some notes:
> >>
> >>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
> >>      the review comments I just posted. I expect Jan will be able to 
> >>      fix them up fairly quickly
> >>   
> > 
> > If the first 6 patches of Jan's series are ready to apply, wouldn't it
> > make sense for him to submit that as a separate series?  In the very
> > least, I'd like an Ack from Jan before applying his series partially.
> 
> You have the ack now. I'm very happy that Mark picks this up before it
> started to bitrot too much.
> 
> Mark, do you plan more work in this domain in the next time? I would
> have no problems to route my networking related stuff through one
> coordinating tree, e.g. yours. Besides getting my current queue flushed
> I still have
> 
>  o rework of host_net_redir (requires coordination with Glauber)
How, exactly?

[Qemu-devel] Re: Networking patches queue

[Jan Kiszka]

Glauber Costa wrote:
> On Thu, May 28, 2009 at 05:51:09PM +0200, Jan Kiszka wrote:
>> Anthony Liguori wrote:
>>> Mark McLoughlin wrote:
>>>> Hi Anthony,
>>>>
>>>> Recently, Jan has posted 11 networking patches and I've posted 17, so I
>>>> thought I'd push out a tree with these queued up. Perhaps you want to
>>>> pull from there?
>>>>
>>>> Some notes:
>>>>
>>>>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>>>>      the review comments I just posted. I expect Jan will be able to 
>>>>      fix them up fairly quickly
>>>>   
>>> If the first 6 patches of Jan's series are ready to apply, wouldn't it
>>> make sense for him to submit that as a separate series?  In the very
>>> least, I'd like an Ack from Jan before applying his series partially.
>> You have the ack now. I'm very happy that Mark picks this up before it
>> started to bitrot too much.
>>
>> Mark, do you plan more work in this domain in the next time? I would
>> have no problems to route my networking related stuff through one
>> coordinating tree, e.g. yours. Besides getting my current queue flushed
>> I still have
>>
>>  o rework of host_net_redir (requires coordination with Glauber)
> How, exactly?

By adopting the enhanced hostfwd syntax (allow for host interface
binding). And we need to clean interface for removing and listing. And
thinking about "info hostfwd" (or whatever), I just realized that "info
guestfwd" would be useful too.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: Networking patches queue

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Glauber Costa wrote:
> On Thu, May 28, 2009 at 05:51:09PM +0200, Jan Kiszka wrote:
>> Anthony Liguori wrote:
>>> Mark McLoughlin wrote:
>>>> Hi Anthony,
>>>>
>>>> Recently, Jan has posted 11 networking patches and I've posted 17, so I
>>>> thought I'd push out a tree with these queued up. Perhaps you want to
>>>> pull from there?
>>>>
>>>> Some notes:
>>>>
>>>>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>>>>      the review comments I just posted. I expect Jan will be able to 
>>>>      fix them up fairly quickly
>>>>   
>>> If the first 6 patches of Jan's series are ready to apply, wouldn't it
>>> make sense for him to submit that as a separate series?  In the very
>>> least, I'd like an Ack from Jan before applying his series partially.
>> You have the ack now. I'm very happy that Mark picks this up before it
>> started to bitrot too much.
>>
>> Mark, do you plan more work in this domain in the next time? I would
>> have no problems to route my networking related stuff through one
>> coordinating tree, e.g. yours. Besides getting my current queue flushed
>> I still have
>>
>>  o rework of host_net_redir (requires coordination with Glauber)
> How, exactly?

Oh... forget it. I confused you and Alex /wrt slirp work. He was the one
who recently posted the host_net_redir patches.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Mark McLoughlin wrote:
>
>>  I would
>> have no problems to route my networking related stuff through one
>> coordinating tree, e.g. yours. Besides getting my current queue flushed
>> I still have
>>
>>  o rework of host_net_redir (requires coordination with Glauber)
>>  o multi-instance slirp
>>
>> on my agenda. And maybe more will comes once this is rolled out.
>>     
>
> Cool. We'll see how it goes. Unless Anthony is actually pulling from
> this tree, it's only useful when we have a bunch of inter-dependant
> queued patches.
>   

I'm always happy to pull from trees.  As long as patches have gone to
the list, it no additional work on my part.  If we have a robust test
suite in tree, then trees become very useful in distributing the load. 
Without a good test suite, I have to do a lot of manual tests which
slows down the process.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] Re: Networking patches queue

[Dor Laor]

Anthony Liguori wrote:

..
> Does kvm-autotest have networking tests?
>
>   
Yes and no...
kvm autotest is able to run standard autotest tests - 
http://www.linux-kvm.org/page/KVM-Autotest/Tests#autotest
autotest itself support various tests, netperf2 is one of them. So with 
a little effort it is doable.
We'll try to make it easy as possible to run these tests.
Here's a list of some autotest tests:
aborttest            dacapo            error_test_na  iperf            
lsb_dtk         profiler_test    stress
aio_dio_bugs         dbench            fio            ipv6connect      
ltp             reaim            sysbench
aiostress            dbt2              fsdev          isic             
memory_api      real_time_tests  tbench
barriertest          disktest          fsfuzzer       kernbench        
monotonic_time  rmaptest         tiobench
bash_shared_mapping  error_cleanup     fs_mark        kernelbuild      
netperf2        rttester         tsc
bonnie               error_initialize  fsstress       kvm_runtest_2    
netpipe         scrashme         unixbench
btreplay             error_setup       fsx            kvm_runtest_old  
parallel_dd     selftest         uptime
compilebench         error_skip_step   hackbench      kvmtest          
perfmon         signaltest       xmtest
cpu_hotplug          error_test_bug    interbench     libhugetlbfs     
pi_tests        sleeptest
cpuset_tasks         error_test_error  iosched_bugs   linus_stress     
pktgen          sparse
cyclictest           error_test_fail   iozone         lmbench          
posixtest       spew


> I know there's some concern about the time it takes for patches to get
> applied.  IMHO, the best way to improve that it to get a stronger set of
> functional tests.  I have a set I've been working on but it's on my
> other laptop which I cannot reach ATM :-/
>
> As has been discussed before, I'm looking for finer grain functional
> tests than the installation tests that kvm-autotest is providing now. 
> I'm not saying that this series requires submitting a test suite first,
> but rather that if you're interested in seeing networking advance more
> rapidly, a good investment would be to build out a better test
> infrastructure for it.
>
> Regards,
>
> Anthony Liguori
>
>
>   

[Qemu-devel] Re: Networking patches queue

[Mark McLoughlin]

Hi Anthony,

On Thu, 2009-05-28 at 16:19 +0100, Mark McLoughlin wrote:

> Recently, Jan has posted 11 networking patches and I've posted 17, so I
> thought I'd push out a tree with these queued up. Perhaps you want to
> pull from there?
> 
> Some notes:
> 
>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>      the review comments I just posted. I expect Jan will be able to 
>      fix them up fairly quickly
> 
>    - I've tried my best to fix up the param checking saga by reverting 
>      Kevin's patch, going with Jan's rollback to something closer to 
>      what was there originally and applying a small fixup patch
> 
>    - Not all of these patches are completely isolated to networking 
>      code - e.g. the fork_exec() patch adds a SIGCHLD handler
> 
>    - I haven't reviewed the slirp changes in great detail, but they 
>      look okay at a glance

I've re-based the queue and pulled in Alex's rx filtering patches.

I've tested the tree as follows:

  - Mostly just with --enable-kvm

  - F-11 host and guest

  - Basic functional testing (dhcp, ping, ssh, scp) with tap, slirp, 
    virtio-net and e1000

  - Some quick host<->guest netperf benchmarks (results in Mb/s):

                    | TCP TX | UDP TX | TCP RX | UDP RX
      --------------+--------+--------+--------+-------
      virtio before |  386   |  1545  |   190  |  410 
             after  |  760   |  1540  |  1100  |  860*
      e1000  before |  220   |   155  |    88  |  160
             after  |  400   |   165  |  1255  |  180*

      * - these UDP RX results show the received figures; the sent
          figures are much higher since the host is sending so fast
          it is overflowing the socket buffers in the guest

  - Pushed it through a basic kvm-autotest run, to see if e.g. the 
    SIGCHLD handler had side-effects not thrown up by networking tests

  - Confirmed that invalid parameter and hotplug errors are working as 
    expected after Jan's changes

  - Basic testing of slirp redirs

  - Basic testing of promisc, allmuti, mac table filtering etc. - Alex 
    clearly has tested this in more detail, though

  - Attempted to test old->new virtio-net migrations (given the 
    version_id bumps), but virtio migration is still broken on HEAD

Pull request below.

Cheers,
Mark.

The following changes since commit 98ba2632fc2695838657a972fce69270cb79dc77:
  Gerd Hoffmann (1):
        qdev: c99 initilaizers for bus_type_names

are available in the git repository at:

  git://git.et.redhat.com/qemu-net.git queue

Alex Williamson (7):
      virtio-net: Add version_id 7 placeholder for vnet header support
      virtio-net: Use a byte to store RX mode flags
      virtio-net: reorganize receive_filter()
      virtio-net: Fix MAC filter overflow handling
      virtio-net: MAC filter optimization
      virtio-net: Add new RX filter controls
      virtio-net: Increase filter and control limits

Jan Kiszka (6):
      net: Don't deliver to disabled interfaces in qemu_sendv_packet
      net: Fix and improved ordered packet delivery
      slirp: Avoid zombie processes after fork_exec
      net: Real fix for check_params users
      net: Improve parameter error reporting
      slirp: Reorder initialization

Mark McLoughlin (15):
      Revert "Fix output of uninitialized strings"
      net: fix error reporting for some net parameter checks
      net: factor tap_read_packet() out of tap_send()
      net: move the tap buffer into TAPState
      net: vlan clients with no fd_can_read() can always receive
      net: only read from tapfd when we can send
      net: add fd_readv() handler to qemu_new_vlan_client() args
      net: re-name vc->fd_read() to vc->receive()
      net: pass VLANClientState* as first arg to receive handlers
      net: add return value to packet receive handler
      net: return status from qemu_deliver_packet()
      net: split out packet queueing and flushing into separate functions
      net: add qemu_send_packet_async()
      net: make use of async packet sending API in tap client
      virtio-net: implement rx packet queueing

 hw/dp8393x.c        |   22 +-
 hw/e1000.c          |   30 ++-
 hw/eepro100.c       |   23 +-
 hw/etraxfs_eth.c    |   14 +-
 hw/mcf_fec.c        |   11 +-
 hw/mipsnet.c        |   16 +-
 hw/musicpal.c       |   11 +-
 hw/ne2000.c         |   25 +-
 hw/pci-hotplug.c    |    7 +-
 hw/pcnet.c          |   17 +-
 hw/qdev.c           |    9 +-
 hw/rtl8139.c        |   39 ++--
 hw/smc91c111.c      |   18 +-
 hw/stellaris_enet.c |   20 +-
 hw/usb-net.c        |   18 +-
 hw/virtio-net.c     |  154 +++++++++---
 hw/virtio-net.h     |   14 +-
 hw/xen_nic.c        |   26 +-
 net.c               |  708 ++++++++++++++++++++++++++++++++++-----------------
 net.h               |   31 ++-
 savevm.c            |    2 +-
 slirp/libslirp.h    |    2 +-
 slirp/slirp.c       |    2 +-
 sysemu.h            |    3 +-
 tap-win32.c         |    8 +-
 vl.c                |   57 ++--
 26 files changed, 834 insertions(+), 453 deletions(-)

[Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Mark McLoughlin wrote:
> I've re-based the queue and pulled in Alex's rx filtering patches.
>
> I've tested the tree as follows:
>
>   - Mostly just with --enable-kvm
>
>   - F-11 host and guest
>
>   - Basic functional testing (dhcp, ping, ssh, scp) with tap, slirp, 
>     virtio-net and e1000
>
>   - Some quick host<->guest netperf benchmarks (results in Mb/s):
>
>                     | TCP TX | UDP TX | TCP RX | UDP RX
>       --------------+--------+--------+--------+-------
>       virtio before |  386   |  1545  |   190  |  410 
>              after  |  760   |  1540  |  1100  |  860*
>       e1000  before |  220   |   155  |    88  |  160
>              after  |  400   |   165  |  1255  |  180*
>
>       * - these UDP RX results show the received figures; the sent
>           figures are much higher since the host is sending so fast
>           it is overflowing the socket buffers in the guest
>
>   - Pushed it through a basic kvm-autotest run, to see if e.g. the 
>     SIGCHLD handler had side-effects not thrown up by networking tests
>
>   - Confirmed that invalid parameter and hotplug errors are working as 
>     expected after Jan's changes
>
>   - Basic testing of slirp redirs
>
>   - Basic testing of promisc, allmuti, mac table filtering etc. - Alex 
>     clearly has tested this in more detail, though
>
>   - Attempted to test old->new virtio-net migrations (given the 
>     version_id bumps), but virtio migration is still broken on HEAD
>
> Pull request below.
>   

Pulled.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Mark McLoughlin wrote:
> Hi Anthony,
>
> On Thu, 2009-05-28 at 16:19 +0100, Mark McLoughlin wrote:
>
>   
>> Recently, Jan has posted 11 networking patches and I've posted 17, so I
>> thought I'd push out a tree with these queued up. Perhaps you want to
>> pull from there?
>>
>> Some notes:
>>
>>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
>>      the review comments I just posted. I expect Jan will be able to 
>>      fix them up fairly quickly
>>
>>    - I've tried my best to fix up the param checking saga by reverting 
>>      Kevin's patch, going with Jan's rollback to something closer to 
>>      what was there originally and applying a small fixup patch
>>
>>    - Not all of these patches are completely isolated to networking 
>>      code - e.g. the fork_exec() patch adds a SIGCHLD handler
>>
>>    - I haven't reviewed the slirp changes in great detail, but they 
>>      look okay at a glance
>>     
>
> I've re-based the queue and pulled in Alex's rx filtering patches.
>
> I've tested the tree as follows:
>
>   - Mostly just with --enable-kvm
>
>   - F-11 host and guest
>
>   - Basic functional testing (dhcp, ping, ssh, scp) with tap, slirp, 
>     virtio-net and e1000
>
>   - Some quick host<->guest netperf benchmarks (results in Mb/s):
>
>                     | TCP TX | UDP TX | TCP RX | UDP RX
>       --------------+--------+--------+--------+-------
>       virtio before |  386   |  1545  |   190  |  410 
>              after  |  760   |  1540  |  1100  |  860*
>       e1000  before |  220   |   155  |    88  |  160
>              after  |  400   |   165  |  1255  |  180*
>
>       * - these UDP RX results show the received figures; the sent
>           figures are much higher since the host is sending so fast
>           it is overflowing the socket buffers in the guest
>
>   - Pushed it through a basic kvm-autotest run, to see if e.g. the 
>     SIGCHLD handler had side-effects not thrown up by networking tests
>
>   - Confirmed that invalid parameter and hotplug errors are working as 
>     expected after Jan's changes
>
>   - Basic testing of slirp redirs
>
>   - Basic testing of promisc, allmuti, mac table filtering etc. - Alex 
>     clearly has tested this in more detail, though
>
>   - Attempted to test old->new virtio-net migrations (given the 
>     version_id bumps), but virtio migration is still broken on HEAD
>
> Pull request below.
>
> Cheers,
> Mark.
>
> The following changes since commit 98ba2632fc2695838657a972fce69270cb79dc77:
>   Gerd Hoffmann (1):
>         qdev: c99 initilaizers for bus_type_names
>
> are available in the git repository at:
>
>   git://git.et.redhat.com/qemu-net.git queue
>
> Alex Williamson (7):
>       virtio-net: Add version_id 7 placeholder for vnet header support
>       virtio-net: Use a byte to store RX mode flags
>       virtio-net: reorganize receive_filter()
>       virtio-net: Fix MAC filter overflow handling
>       virtio-net: MAC filter optimization
>       virtio-net: Add new RX filter controls
>       virtio-net: Increase filter and control limits
>
> Jan Kiszka (6):
>       net: Don't deliver to disabled interfaces in qemu_sendv_packet
>       net: Fix and improved ordered packet delivery
>       slirp: Avoid zombie processes after fork_exec
>       net: Real fix for check_params users
>       net: Improve parameter error reporting
>       slirp: Reorder initialization
>
> Mark McLoughlin (15):
>       Revert "Fix output of uninitialized strings"
>       net: fix error reporting for some net parameter checks
>       net: factor tap_read_packet() out of tap_send()
>       net: move the tap buffer into TAPState
>       net: vlan clients with no fd_can_read() can always receive
>       net: only read from tapfd when we can send
>       net: add fd_readv() handler to qemu_new_vlan_client() args
>       net: re-name vc->fd_read() to vc->receive()
>       net: pass VLANClientState* as first arg to receive handlers
>       net: add return value to packet receive handler
>   

This broke the build with vde enabled.  Because of this line:

+static ssize_t vde_receive(VLANClientState *vc, const uint8_t *buf, 
size_t size)
 {
     VDEState *s = vc->opaque;
-    int ret;
-    for(;;) {
-        ret = vde_send(s->vde, (const char *)buf, size, 0);
-        if (ret < 0 && errno == EINTR) {
-        } else {
-            break;
-        }
-    }
+    ssize ret;

Obvious typo.  I've pushed a fix.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: Networking patches queue

[Mark McLoughlin]

On Wed, 2009-06-10 at 20:27 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > Hi Anthony,
> >
> > On Thu, 2009-05-28 at 16:19 +0100, Mark McLoughlin wrote:
> >
> >   
> >> Recently, Jan has posted 11 networking patches and I've posted 17, so I
> >> thought I'd push out a tree with these queued up. Perhaps you want to
> >> pull from there?
> >>
> >> Some notes:
> >>
> >>    - I've taken the first 6 of Jan's patches, but left 7-11 for now; see
> >>      the review comments I just posted. I expect Jan will be able to 
> >>      fix them up fairly quickly
> >>
> >>    - I've tried my best to fix up the param checking saga by reverting 
> >>      Kevin's patch, going with Jan's rollback to something closer to 
> >>      what was there originally and applying a small fixup patch
> >>
> >>    - Not all of these patches are completely isolated to networking 
> >>      code - e.g. the fork_exec() patch adds a SIGCHLD handler
> >>
> >>    - I haven't reviewed the slirp changes in great detail, but they 
> >>      look okay at a glance
> >>     
> >
> > I've re-based the queue and pulled in Alex's rx filtering patches.
> >
> > I've tested the tree as follows:
> >
> >   - Mostly just with --enable-kvm
> >
> >   - F-11 host and guest
> >
> >   - Basic functional testing (dhcp, ping, ssh, scp) with tap, slirp, 
> >     virtio-net and e1000
> >
> >   - Some quick host<->guest netperf benchmarks (results in Mb/s):
> >
> >                     | TCP TX | UDP TX | TCP RX | UDP RX
> >       --------------+--------+--------+--------+-------
> >       virtio before |  386   |  1545  |   190  |  410 
> >              after  |  760   |  1540  |  1100  |  860*
> >       e1000  before |  220   |   155  |    88  |  160
> >              after  |  400   |   165  |  1255  |  180*
> >
> >       * - these UDP RX results show the received figures; the sent
> >           figures are much higher since the host is sending so fast
> >           it is overflowing the socket buffers in the guest
> >
> >   - Pushed it through a basic kvm-autotest run, to see if e.g. the 
> >     SIGCHLD handler had side-effects not thrown up by networking tests
> >
> >   - Confirmed that invalid parameter and hotplug errors are working as 
> >     expected after Jan's changes
> >
> >   - Basic testing of slirp redirs
> >
> >   - Basic testing of promisc, allmuti, mac table filtering etc. - Alex 
> >     clearly has tested this in more detail, though
> >
> >   - Attempted to test old->new virtio-net migrations (given the 
> >     version_id bumps), but virtio migration is still broken on HEAD
> >
> > Pull request below.
> >
> > Cheers,
> > Mark.
> >
> > The following changes since commit 98ba2632fc2695838657a972fce69270cb79dc77:
> >   Gerd Hoffmann (1):
> >         qdev: c99 initilaizers for bus_type_names
> >
> > are available in the git repository at:
> >
> >   git://git.et.redhat.com/qemu-net.git queue
> >
> > Alex Williamson (7):
> >       virtio-net: Add version_id 7 placeholder for vnet header support
> >       virtio-net: Use a byte to store RX mode flags
> >       virtio-net: reorganize receive_filter()
> >       virtio-net: Fix MAC filter overflow handling
> >       virtio-net: MAC filter optimization
> >       virtio-net: Add new RX filter controls
> >       virtio-net: Increase filter and control limits
> >
> > Jan Kiszka (6):
> >       net: Don't deliver to disabled interfaces in qemu_sendv_packet
> >       net: Fix and improved ordered packet delivery
> >       slirp: Avoid zombie processes after fork_exec
> >       net: Real fix for check_params users
> >       net: Improve parameter error reporting
> >       slirp: Reorder initialization
> >
> > Mark McLoughlin (15):
> >       Revert "Fix output of uninitialized strings"
> >       net: fix error reporting for some net parameter checks
> >       net: factor tap_read_packet() out of tap_send()
> >       net: move the tap buffer into TAPState
> >       net: vlan clients with no fd_can_read() can always receive
> >       net: only read from tapfd when we can send
> >       net: add fd_readv() handler to qemu_new_vlan_client() args
> >       net: re-name vc->fd_read() to vc->receive()
> >       net: pass VLANClientState* as first arg to receive handlers
> >       net: add return value to packet receive handler
> >   
> 
> This broke the build with vde enabled.  Because of this line:
> 
> +static ssize_t vde_receive(VLANClientState *vc, const uint8_t *buf, 
> size_t size)
>  {
>      VDEState *s = vc->opaque;
> -    int ret;
> -    for(;;) {
> -        ret = vde_send(s->vde, (const char *)buf, size, 0);
> -        if (ret < 0 && errno == EINTR) {
> -        } else {
> -            break;
> -        }
> -    }
> +    ssize ret;
> 
> Obvious typo.  I've pushed a fix.

Gah, my apologies - I'll make sure to build with vde in future.

Thanks,
Mark.

Re: [Qemu-devel] Re: Networking patches queue

[Gerd Hoffmann]

   Hi,

>    git://git.et.redhat.com/qemu-net.git queue

Breaks full build (i.e. all targets).

cheers,
   Gerd

[Qemu-devel] [PATCH] Fix xilinx_ethlite breakage by 4f1c942b7f

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

Gerd Hoffmann wrote:
>   Hi,
> 
>>    git://git.et.redhat.com/qemu-net.git queue
> 
> Breaks full build (i.e. all targets).
> 

Namely the new xilinx_ethlite used by mircoblaze. This fixes it for me,
hopefully in the right way.

Jan

------------>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 hw/xilinx_ethlite.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/xilinx_ethlite.c b/hw/xilinx_ethlite.c
index 780e9c0..77cd6fb 100644
--- a/hw/xilinx_ethlite.c
+++ b/hw/xilinx_ethlite.c
@@ -160,28 +160,28 @@ static CPUWriteMemoryFunc *eth_write[] = {
     NULL, NULL, &eth_writel,
 };
 
-static int eth_can_rx(void *opaque)
+static int eth_can_rx(VLANClientState *vc)
 {
-    struct xlx_ethlite *s = opaque;
+    struct xlx_ethlite *s = vc->opaque;
     int r;
     r = !(s->regs[R_RX_CTRL0] & CTRL_S);
     qemu_log("%s %d\n", __func__, r);
     return r;
 }
 
-static void eth_rx(void *opaque, const uint8_t *buf, int size)
+static ssize_t eth_rx(VLANClientState *vc, const uint8_t *buf, size_t size)
 {
-    struct xlx_ethlite *s = opaque;
+    struct xlx_ethlite *s = vc->opaque;
     unsigned int rxbase = s->rxbuf * (0x800 / 4);
     int i;
 
     /* DA filter.  */
     if (!(buf[0] & 0x80) && memcmp(&s->macaddr[0], buf, 6))
-        return;
+        return size;
 
     if (s->regs[rxbase + R_RX_CTRL0] & CTRL_S) {
         D(qemu_log("ethlite lost packet %x\n", s->regs[R_RX_CTRL0]));
-        return;
+        return -1;
     }
 
     D(qemu_log("%s %d rxbase=%x\n", __func__, size, rxbase));
@@ -199,7 +199,7 @@ static void eth_rx(void *opaque, const uint8_t *buf, int size)
 
     /* If c_rx_pingpong was set flip buffers.  */
     s->rxbuf ^= s->c_rx_pingpong;
-    return;
+    return size;
 }
 
 static void eth_cleanup(VLANClientState *vc)
@@ -223,7 +223,7 @@ static void xilinx_ethlite_init(SysBusDevice *dev)
 
     qdev_get_macaddr(&dev->qdev, s->macaddr);
     s->vc = qdev_get_vlan_client(&dev->qdev,
-                                 eth_rx, eth_can_rx, eth_cleanup, s);
+                                 eth_can_rx, eth_rx, NULL, eth_cleanup, s);
 }
 
 static void xilinx_ethlite_register(void)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH] Fix xilinx_ethlite breakage by 4f1c942b7f

[Mark McLoughlin]

On Thu, 2009-06-11 at 11:42 +0200, Jan Kiszka wrote:
> Gerd Hoffmann wrote:
> >   Hi,
> > 
> >>    git://git.et.redhat.com/qemu-net.git queue
> > 
> > Breaks full build (i.e. all targets).

Gah - I built the first pull-request on all targets, but neglected to do
it for the resend. My bad.

> Namely the new xilinx_ethlite used by mircoblaze. This fixes it for me,
> hopefully in the right way.

Identical to what I was about to send, thanks

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Acked-by: Mark McLoughlin <markmc@redhat.com>

Thanks,
Mark.

Re: [Qemu-devel] Re: Networking patches queue

[Paul Brook]

On Tuesday 09 June 2009, Mark McLoughlin wrote:
>       net: add return value to packet receive handler (4f1c942b7f)

This patch is wrong. We already have a can_receive handler for this purpose.  
Having this callback and allowing receive to fail is just silly. One or the 
other, but not both.

Paul

Re: [Qemu-devel] Re: Networking patches queue

[Anthony Liguori]

Paul Brook wrote:
> On Tuesday 09 June 2009, Mark McLoughlin wrote:
>   
>>       net: add return value to packet receive handler (4f1c942b7f)
>>     
>
> This patch is wrong. We already have a can_receive handler for this purpose.  
> Having this callback and allowing receive to fail is just silly. One or the 
> other, but not both.
>   

How can can_receive fail for something like tap without actually trying 
to receive a packet?

If anything, we should just get rid of can_receive.

-- 
Regards,

Anthony Liguori