> On 8 Mar 2016, at 05:01 AM, Jason Wang <[email protected]> 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. > >> >> 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.
No problem, we added a unit test into the next version of patches. > > 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
