qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 00/13] Introduce Intel 82574 GbE Controller E


From: Dmitry Fleytman
Subject: Re: [Qemu-devel] [PATCH v2 00/13] Introduce Intel 82574 GbE Controller Emulation (e1000e)
Date: Wed, 6 Apr 2016 10:25:15 +0300


On 8 Mar 2016, at 05:01 AM, Jason Wang <address@hidden> wrote:



On 02/23/2016 01:37 AM, Leonid Bloch wrote:
Hello All,

This is v2 of the patches, after the initial reviews.

For convenience, the same patches are available at:
https://github.com/daynix/qemu-e1000e/tree/e1000e-submit-v2

Best regards,
Leonid.

Changes since v1:

1. PCI_PM_CAP_VER_1_1 is defined now in include/hw/pci/pci_regs.h and
  not in include/standard-headers/linux/pci_regs.h.
2. Changes in naming and extra comments in hw/pci/pcie.c and in
  include/hw/pci/pcie.h.
3. Defining pci_dsn_ver and pci_dsn_cap static const variables in
  hw/pci/pcie.c, instead of PCI_DSN_VER and PCI_DSN_CAP symbolic
  constants in include/hw/pci/pcie_regs.h.
4. Changing the vmxnet3_device_serial_num function in hw/net/vmxnet3.c
  to avoid the cast when it is called.
5. Avoiding a preceding underscore in all the e1000e-related names.
6. Minor style changes.

===================

Hello All,

This series is the final code of the e1000e device emulation, that we
have developed. Please review, and consider acceptance of these patches
to the upstream QEMU repository.

The code stability was verified by various traffic tests using Fedora 22
Linux, and Windows Server 2012R2 guests. Also, Microsoft Hardware
Certification Kit (HCK) tests were run on a Windows Server 2012R2 guest.

Sounds really cool.


There was a discussion on the possibility of code sharing between the
e1000e, and the existing e1000 devices. We have reviewed the final code
for parts that may be shared between this device and the currently
available e1000 emulation. The device specifications are very different,
and there are almost no registers, nor functions, that were left as is
from e1000. The ring descriptor structures were changed as well, by the
introduction of extended and PS descriptors, as well as additional bits.

How about sharing the codes at function level somehow, e.g
e1000e_receive_filter() and receive_filter() in e1000, the only
difference is e1000's version has tracing.

Sure. We are introducing code sharing in the next patch version.



Additional differences stem from the fact that the e1000e device re-uses
network packet abstractions introduced by the vmxnet3 device, while the
e1000 has its own code for packet handling. BTW, it may be worth reusing
those abstractions in e1000 as well. 

Right, but we have the risk to break e1000.

(Following these changes the
vmxnet3 device was successfully tested for possible regressions.)

There are a few minor parts that may be shared, e.g. the default
register handlers, and the ring management functions. The total amount
of shared lines will be about 100--150, so we're not sure if it makes
sense bothering, and taking a risk of breaking e1000, which is a good,
old, and stable device.

Right, but if the changes in e1000 are small and direct, it may be
worth. I think we don't want to fix bug in two places just because of
codes duplications.


Currently, the e1000e code is stand alone w.r.t. e1000.

Please share your thoughts.

The patch looks good, see individual patches for comments.

One thing I'd expect to add is unit-test. Let's add one to test basic
tx/rx function for e1000e, you may want to refer tests/virtio-net-test.c
for an example.

Thanks


Thanks in advance,
Dmitry.


Changes since RFCv2:

1. Device functionality verified using Microsoft Hardware Certification Test Kit (HCK)
2. Introduced a number of performance improvements
3. The code was cleaned, and rebased to the latest master
4. Patches verified with checkpatch.pl

===================

Changes since RFCv1:

1. Added support for all the device features:
 - Interrupt moderation.
 - RSS.
 - Multiqueue.
2. Simulated exact PCI/PCIe configuration space layout.
3. Made fixes needed to pass Microsoft's HW certification tests (HCK).

This series is still an RFC, because the following tasks are not done yet:

1. See which code can be shared between this device and the existing e1000 device.
2. Rebase patches to the latest master (current base is v2.3.0).

Please share your thoughts,
Thanks, Dmitry.

===================

Hello qemu-devel,

This patch series is an RFC for the new networking device emulation
we're developing for QEMU.

This new device emulates the Intel 82574 GbE Controller and works
with unmodified Intel e1000e drivers from the Linux/Windows kernels.

The status of the current series is "Functional Device Ready, work
on Extended Features in Progress".

More precisely, these patches represent a functional device, which
is recognized by the standard Intel drivers, and is able to transfer
TX/RX packets with CSO/TSO offloads, according to the spec.

Extended features not supported yet (work in progress):
 1. TX/RX Interrupt moderation mechanisms
 2. RSS
 3. Full-featured multi-queue (use of multiqueued network backend)

Also, there will be some code refactoring and performance
optimization efforts.

This series was tested on Linux (Fedora 22) and Windows (2012R2)
guests, using Iperf, with TX/RX and TCP/UDP streams, and various
packet sizes.

More thorough testing, including data streams with different MTU
sizes, and Microsoft Certification (HLK) tests, are pending missing
features' development.

See commit messages (esp. "net: Introduce e1000e device emulation")
for more information about the development approaches and the
architecture options chosen for this device.

This series is based upon v2.3.0 tag of the upstream QEMU repository,
and it will be rebased to latest before the final submission.

Please share your thoughts - any feedback is highly welcomed :)

Best Regards,
Dmitry Fleytman.

Dmitry Fleytman (13):
 msix: make msix_clr_pending() visible for clients
 pci: Introduce define for PM capability version 1.1
 pcie: Add support for PCIe CAP v1
 pcie: Introduce function for DSN capability creation
 vmxnet3: Use generic function for DSN capability definition
 net: Introduce Toeplitz hash calculator
 net: Add macros for MAC address tracing
 vmxnet3: Use common MAC address tracing macros
 net_pkt: Name vmxnet3 packet abstractions more generic
 rtl8139: Move more TCP definitions to common header
 net_pkt: Extend packet abstraction as required by e1000e functionality
 e1000_regs: Add definitions for Intel 82574-specific bits
 net: Introduce e1000e device emulation

MAINTAINERS                |   14 +
default-configs/pci.mak    |    1 +
docs/specs/e1000e-spec.txt |   17 +
hw/net/Makefile.objs       |    5 +-
hw/net/e1000_regs.h        |  351 ++++-
hw/net/e1000e.c            |  718 +++++++++
hw/net/e1000e_core.c       | 3673 ++++++++++++++++++++++++++++++++++++++++++++
hw/net/e1000e_core.h       |  230 +++
hw/net/net_rx_pkt.c        |  599 ++++++++
hw/net/net_rx_pkt.h        |  365 +++++
hw/net/net_tx_pkt.c        |  631 ++++++++
hw/net/net_tx_pkt.h        |  191 +++
hw/net/rtl8139.c           |    5 -
hw/net/vmxnet3.c           |  108 +-
hw/net/vmxnet_debug.h      |    3 -
hw/net/vmxnet_rx_pkt.c     |  187 ---
hw/net/vmxnet_rx_pkt.h     |  176 ---
hw/net/vmxnet_tx_pkt.c     |  581 -------
hw/net/vmxnet_tx_pkt.h     |  148 --
hw/pci/msix.c              |    2 +-
hw/pci/pcie.c              |   94 +-
include/hw/pci/msix.h      |    1 +
include/hw/pci/pci_regs.h  |    2 +
include/hw/pci/pcie.h      |    5 +
include/hw/pci/pcie_regs.h |    5 +-
include/net/checksum.h     |   49 +-
include/net/eth.h          |  161 +-
include/net/net.h          |    5 +
net/checksum.c             |    7 +-
net/eth.c                  |  410 ++++-
tests/Makefile             |    4 +-
trace-events               |  210 +++
32 files changed, 7681 insertions(+), 1277 deletions(-)
create mode 100644 docs/specs/e1000e-spec.txt
create mode 100644 hw/net/e1000e.c
create mode 100644 hw/net/e1000e_core.c
create mode 100644 hw/net/e1000e_core.h
create mode 100644 hw/net/net_rx_pkt.c
create mode 100644 hw/net/net_rx_pkt.h
create mode 100644 hw/net/net_tx_pkt.c
create mode 100644 hw/net/net_tx_pkt.h
delete mode 100644 hw/net/vmxnet_rx_pkt.c
delete mode 100644 hw/net/vmxnet_rx_pkt.h
delete mode 100644 hw/net/vmxnet_tx_pkt.c
delete mode 100644 hw/net/vmxnet_tx_pkt.h


reply via email to

[Prev in Thread] Current Thread [Next in Thread]