> -----Original Message----- > From: Akihiko Odaki <akihiko.od...@daynix.com> > Sent: Monday, 30 January 2023 15:39 > 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/29 5:57, Sriram Yagnaraman wrote: > >> -----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). > > Thanks for your work and responding to my comments. Please check the > comments for the latest series I have just sent, and also rebase it to the > latest > version of this series.
Done now. Thanks for the review, and really great work with this patch to introduce igb. > > > > >> > >> 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? > > igb patches are now squahed in the latest version, which I have just sent. I have reviewed and put a Reviewed-By tag for the igb patch. I hope it is OK for me to do that, since I am new to qemu-devel. > > > > >> > >> 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 > >>>>>>> > >>>>>>
