> -----Original Message----- > From: Akihiko Odaki <akihiko.od...@daynix.com> > Sent: Thursday, 26 January 2023 12:32 > To: Sriram Yagnaraman <sriram.yagnara...@est.tech>; Jason Wang > <jasow...@redhat.com> > Cc: Dmitry Fleytman <dmitry.fleyt...@gmail.com>; Michael S. Tsirkin > <m...@redhat.com>; Marcel Apfelbaum <marcel.apfelb...@gmail.com>; > Alex Bennée <alex.ben...@linaro.org>; Philippe Mathieu-Daudé > <phi...@linaro.org>; Thomas Huth <th...@redhat.com>; Wainer dos Santos > Moschetta <waine...@redhat.com>; Beraldo Leal <bl...@redhat.com>; > Cleber Rosa <cr...@redhat.com>; Laurent Vivier <lviv...@redhat.com>; > Paolo Bonzini <pbonz...@redhat.com>; Alexander Bulekov <alx...@bu.edu>; > Bandan Das <b...@redhat.com>; Stefan Hajnoczi <stefa...@redhat.com>; > Darren Kenny <darren.ke...@oracle.com>; Qiuhao Li > <qiuhao...@outlook.com>; qemu-devel@nongnu.org; qemu- > p...@nongnu.org; de...@daynix.com; Yan Vugenfirer > <yvuge...@redhat.com>; Yuri Benditovich <yuri.benditov...@daynix.com> > Subject: Re: [PATCH v2 00/13] Introduce igb > > On 2023/01/26 18:34, Sriram Yagnaraman wrote: > > > >> -----Original Message----- > >> From: Sriram Yagnaraman > >> Sent: Tuesday, 24 January 2023 09:54 > >> To: Akihiko Odaki <akihiko.od...@daynix.com>; Jason Wang > >> <jasow...@redhat.com> > >> Cc: Dmitry Fleytman <dmitry.fleyt...@gmail.com>; Michael S. Tsirkin > >> <m...@redhat.com>; Marcel Apfelbaum <marcel.apfelb...@gmail.com>; > Alex > >> Bennée <alex.ben...@linaro.org>; Philippe Mathieu-Daudé > >> <phi...@linaro.org>; Thomas Huth <th...@redhat.com>; Wainer dos > >> Santos Moschetta <waine...@redhat.com>; Beraldo Leal > >> <bl...@redhat.com>; Cleber Rosa <cr...@redhat.com>; Laurent Vivier > >> <lviv...@redhat.com>; Paolo Bonzini <pbonz...@redhat.com>; Alexander > >> Bulekov <alx...@bu.edu>; Bandan Das <b...@redhat.com>; Stefan > Hajnoczi > >> <stefa...@redhat.com>; Darren Kenny <darren.ke...@oracle.com>; > Qiuhao > >> Li <qiuhao...@outlook.com>; qemu-devel@nongnu.org; qemu- > >> p...@nongnu.org; de...@daynix.com; Yan Vugenfirer > >> <yvuge...@redhat.com>; Yuri Benditovich <yuri.benditov...@daynix.com> > >> Subject: RE: [PATCH v2 00/13] Introduce igb > >> > >> > >>> -----Original Message----- > >>> From: Akihiko Odaki <akihiko.od...@daynix.com> > >>> Sent: Tuesday, 24 January 2023 05:54 > >>> To: Jason Wang <jasow...@redhat.com>; Sriram Yagnaraman > >>> <sriram.yagnara...@est.tech> > >>> Cc: Dmitry Fleytman <dmitry.fleyt...@gmail.com>; Michael S. Tsirkin > >>> <m...@redhat.com>; Marcel Apfelbaum > <marcel.apfelb...@gmail.com>; > >> Alex > >>> Bennée <alex.ben...@linaro.org>; Philippe Mathieu-Daudé > >>> <phi...@linaro.org>; Thomas Huth <th...@redhat.com>; Wainer dos > >> Santos > >>> Moschetta <waine...@redhat.com>; Beraldo Leal <bl...@redhat.com>; > >>> Cleber Rosa <cr...@redhat.com>; Laurent Vivier <lviv...@redhat.com>; > >>> Paolo Bonzini <pbonz...@redhat.com>; Alexander Bulekov > >>> <alx...@bu.edu>; Bandan Das <b...@redhat.com>; Stefan Hajnoczi > >>> <stefa...@redhat.com>; Darren Kenny <darren.ke...@oracle.com>; > >> Qiuhao > >>> Li <qiuhao...@outlook.com>; qemu-devel@nongnu.org; qemu- > >>> p...@nongnu.org; de...@daynix.com; Yan Vugenfirer > >>> <yvuge...@redhat.com>; Yuri Benditovich > >>> <yuri.benditov...@daynix.com> > >>> Subject: Re: [PATCH v2 00/13] Introduce igb > >>> > >>> On 2023/01/16 17:01, Jason Wang wrote: > >>>> On Sat, Jan 14, 2023 at 12:10 PM Akihiko Odaki > >>> <akihiko.od...@daynix.com> wrote: > >>>>> > >>>>> Based-on: <20230114035919.35251-1-akihiko.od...@daynix.com> > >>>>> ([PATCH 00/19] e1000x cleanups (preliminary for IGB)) > >>>>> > >>>>> igb is a family of Intel's gigabit ethernet controllers. This > >>>>> series implements > >>>>> 82576 emulation in particular. You can see the last patch for the > >>> documentation. > >>>>> > >>>>> Note that there is another effort to bring 82576 emulation. This > >>>>> series was developed independently by Sriram Yagnaraman. > >>>>> https://lists.gnu.org/archive/html/qemu-devel/2022- > 12/msg04670.htm > >>>>> l > >>>>> > >>>>> It is possible to merge the work from Sriram Yagnaraman and to > >>>>> cherry-pick useful changes from this series later. > >>>>> > >>>>> I think there are several different ways to get the changes into > >>>>> the > >> mainline. > >>>>> I'm open to any options. > >>>> > >>>> I can only do reviews for the general networking part but not the > >>>> 82576 specific part. It would be better if either of the series can > >>>> get some ACKs from some ones that they are familiar with 82576, > >>>> then I can try to merge. > >>>> > >>>> Thanks > >>> > >>> I have just sent v3 to the list. > >>> > >>> Sriram Yagnaraman, who wrote another series for 82576, is the only > >>> person I know who is familiar with the device. > >>> > >>> Sriram, can you take a look at v3 I have just sent? > >> > >> I am at best a good interpreter of the 82576 datasheet. I will review > >> your changes get back here. > > > > I have reviewed and tested your changes and it looks great to me in general. > > I would like to note some features that I would like to add on top of > > your patch, if you have not worked on these already :) > > - PFRSTD (PF reset done) > > - SRRCTL (Rx desc buf size) > > - RLPML (oversized packet handling) > > - MAC/VLAN anti-spoof checks > > - VMOLR_STRVLAN and RPLOLR_STRVLAN (VLAN stripping for VFs) > > - VMVIR (VLAN insertion for VFs) > > - VF reset > > - VFTE, VFRE, VFLRE > > - VF stats > > - Set EITR initial value > > > > Since this is a new device and there are no existing users, is it possible > > to get > the change into baseline first and fix missing features and bugs soon after? > > Thanks for reviewing, > > I have just submitted v4. The difference from v3 is only that igb now > correctly > specifies VFs associated with queues for DMA. > > RX descriptor buffer size in SRRCTL is respected since v3. I think the other > features are missing. I am not planning to implement them either, but I'm > considering to test the code with DPDK and I may add features it requires.
Ok, I just sent a patchset adding most of the features I listed above ([PATCH 0/9] igb: add missing feature set). > > I also want to get this series into the mainline before adding new features > as it > is already so big, but please tell me if you noticed bugs, especially ones > which > can be fixed without adding more code. LGTM, I have tested your changes and it works perfectly. Thank you. Is it possible to squash your igb commits into one patch that I can give an ACK to? > > Regards, > Akihiko Odaki > > > > >> > >>> > >>> Regards, > >>> Akihiko Odaki > >>> > >>>> > >>>>> > >>>>> V1 -> V2: > >>>>> - Spun off e1000e general improvements to a distinct series. > >>>>> - Restored vnet_hdr offload as there seems nothing preventing from > that. > >>>>> > >>>>> Akihiko Odaki (13): > >>>>> hw/net/net_tx_pkt: Introduce net_tx_pkt_get_eth_hdr > >>>>> pcie: Introduce pcie_sriov_num_vfs > >>>>> e1000: Split header files > >>>>> igb: Copy e1000e code > >>>>> igb: Rename identifiers > >>>>> igb: Build igb > >>>>> igb: Transform to 82576 implementation > >>>>> tests/qtest/e1000e-test: Fabricate ethernet header > >>>>> tests/qtest/libqos/e1000e: Export macreg functions > >>>>> tests/qtest/libqos/igb: Copy e1000e code > >>>>> tests/qtest/libqos/igb: Transform to igb tests > >>>>> tests/avocado: Add igb test > >>>>> docs/system/devices/igb: Add igb documentation > >>>>> > >>>>> MAINTAINERS | 9 + > >>>>> docs/system/device-emulation.rst | 1 + > >>>>> docs/system/devices/igb.rst | 70 + > >>>>> hw/net/Kconfig | 5 + > >>>>> hw/net/e1000.c | 1 + > >>>>> hw/net/e1000_common.h | 102 + > >>>>> hw/net/e1000_regs.h | 927 +--- > >>>>> hw/net/e1000e.c | 3 +- > >>>>> hw/net/e1000e_core.c | 1 + > >>>>> hw/net/e1000x_common.c | 1 + > >>>>> hw/net/e1000x_common.h | 74 - > >>>>> hw/net/e1000x_regs.h | 940 ++++ > >>>>> hw/net/igb.c | 615 +++ > >>>>> hw/net/igb_common.h | 144 + > >>>>> hw/net/igb_core.c | 3946 > >>>>> +++++++++++++++++ > >>>>> hw/net/igb_core.h | 147 + > >>>>> hw/net/igb_regs.h | 624 +++ > >>>>> hw/net/igbvf.c | 327 ++ > >>>>> hw/net/meson.build | 2 + > >>>>> hw/net/net_tx_pkt.c | 6 + > >>>>> hw/net/net_tx_pkt.h | 8 + > >>>>> hw/net/trace-events | 32 + > >>>>> hw/pci/pcie_sriov.c | 5 + > >>>>> include/hw/pci/pcie_sriov.h | 3 + > >>>>> .../org.centos/stream/8/x86_64/test-avocado | 1 + > >>>>> tests/avocado/igb.py | 38 + > >>>>> tests/qtest/e1000e-test.c | 17 +- > >>>>> tests/qtest/fuzz/generic_fuzz_configs.h | 5 + > >>>>> tests/qtest/igb-test.c | 243 + > >>>>> tests/qtest/libqos/e1000e.c | 12 - > >>>>> tests/qtest/libqos/e1000e.h | 14 + > >>>>> tests/qtest/libqos/igb.c | 185 + > >>>>> tests/qtest/libqos/meson.build | 1 + > >>>>> tests/qtest/meson.build | 1 + > >>>>> 34 files changed, 7492 insertions(+), 1018 deletions(-) > >>>>> create mode 100644 docs/system/devices/igb.rst > >>>>> create mode 100644 hw/net/e1000_common.h > >>>>> create mode 100644 hw/net/e1000x_regs.h > >>>>> create mode 100644 hw/net/igb.c > >>>>> create mode 100644 hw/net/igb_common.h > >>>>> create mode 100644 hw/net/igb_core.c > >>>>> create mode 100644 hw/net/igb_core.h > >>>>> create mode 100644 hw/net/igb_regs.h > >>>>> create mode 100644 hw/net/igbvf.c > >>>>> create mode 100644 tests/avocado/igb.py > >>>>> create mode 100644 tests/qtest/igb-test.c > >>>>> create mode 100644 tests/qtest/libqos/igb.c > >>>>> > >>>>> -- > >>>>> 2.39.0 > >>>>> > >>>>
