RE: [PATCH v2 00/13] Introduce igb

Sriram Yagnaraman posted 13 patches 1 year, 2 months ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH v2 00/13] Introduce igb
Posted by Sriram Yagnaraman 1 year, 2 months ago
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Tuesday, 24 January 2023 05:54
> To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
> <sriram.yagnaraman@est.tech>
> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com> wrote:
> >>
> >> Based-on: <20230114035919.35251-1-akihiko.odaki@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.html
> >>
> >> 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.

> 
> 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
> >>
> >
RE: [PATCH v2 00/13] Introduce igb
Posted by Sriram Yagnaraman 1 year, 2 months ago
> -----Original Message-----
> From: Sriram Yagnaraman
> Sent: Tuesday, 24 January 2023 09:54
> To: Akihiko Odaki <akihiko.odaki@daynix.com>; Jason Wang
> <jasowang@redhat.com>
> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@daynix.com>
> Subject: RE: [PATCH v2 00/13] Introduce igb
> 
> 
> > -----Original Message-----
> > From: Akihiko Odaki <akihiko.odaki@daynix.com>
> > Sent: Tuesday, 24 January 2023 05:54
> > To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
> > <sriram.yagnaraman@est.tech>
> > Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> > <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex
> > Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> > <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> Santos
> > Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> > Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> > Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov
> > <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan Hajnoczi
> > <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
> Qiuhao
> > Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> > ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> > <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com> wrote:
> > >>
> > >> Based-on: <20230114035919.35251-1-akihiko.odaki@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.html
> > >>
> > >> 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?

> 
> >
> > 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
> > >>
> > >
Re: [PATCH v2 00/13] Introduce igb
Posted by Akihiko Odaki 1 year, 2 months ago
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.odaki@daynix.com>; Jason Wang
>> <jasowang@redhat.com>
>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
>> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
>> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
>> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
>> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
>> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
>> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@daynix.com>
>> Subject: RE: [PATCH v2 00/13] Introduce igb
>>
>>
>>> -----Original Message-----
>>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Sent: Tuesday, 24 January 2023 05:54
>>> To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
>>> <sriram.yagnaraman@est.tech>
>>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
>>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Alex
>>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
>>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
>> Santos
>>> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
>>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
>>> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov
>>> <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan Hajnoczi
>>> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
>> Qiuhao
>>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
>>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
>>> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com> wrote:
>>>>>
>>>>> Based-on: <20230114035919.35251-1-akihiko.odaki@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.html
>>>>>
>>>>> 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.

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.

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
>>>>>
>>>>

RE: [PATCH v2 00/13] Introduce igb
Posted by Sriram Yagnaraman 1 year, 1 month ago
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Thursday, 26 January 2023 12:32
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
> <jasowang@redhat.com>
> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com>; Jason Wang
> >> <jasowang@redhat.com>
> >> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> >> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex
> >> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> >> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> >> Santos Moschetta <wainersm@redhat.com>; Beraldo Leal
> >> <bleal@redhat.com>; Cleber Rosa <crosa@redhat.com>; Laurent Vivier
> >> <lvivier@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Alexander
> >> Bulekov <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan
> Hajnoczi
> >> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
> Qiuhao
> >> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> >> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> >> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@daynix.com>
> >> Subject: RE: [PATCH v2 00/13] Introduce igb
> >>
> >>
> >>> -----Original Message-----
> >>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> >>> Sent: Tuesday, 24 January 2023 05:54
> >>> To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
> >>> <sriram.yagnaraman@est.tech>
> >>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> >>> <mst@redhat.com>; Marcel Apfelbaum
> <marcel.apfelbaum@gmail.com>;
> >> Alex
> >>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> >>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> >> Santos
> >>> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> >>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> >>> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov
> >>> <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan Hajnoczi
> >>> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
> >> Qiuhao
> >>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> >>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> >>> <yvugenfi@redhat.com>; Yuri Benditovich
> >>> <yuri.benditovich@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.odaki@daynix.com> wrote:
> >>>>>
> >>>>> Based-on: <20230114035919.35251-1-akihiko.odaki@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
> >>>>>
> >>>>
Re: [PATCH v2 00/13] Introduce igb
Posted by Akihiko Odaki 1 year, 1 month ago
On 2023/01/29 5:57, Sriram Yagnaraman wrote:
>> -----Original Message-----
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Sent: Thursday, 26 January 2023 12:32
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
>> <jasowang@redhat.com>
>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
>> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
>> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
>> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
>> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
>> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
>> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com>; Jason Wang
>>>> <jasowang@redhat.com>
>>>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
>>>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Alex
>>>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
>>>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
>>>> Santos Moschetta <wainersm@redhat.com>; Beraldo Leal
>>>> <bleal@redhat.com>; Cleber Rosa <crosa@redhat.com>; Laurent Vivier
>>>> <lvivier@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Alexander
>>>> Bulekov <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan
>> Hajnoczi
>>>> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
>> Qiuhao
>>>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
>>>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
>>>> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@daynix.com>
>>>> Subject: RE: [PATCH v2 00/13] Introduce igb
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>>> Sent: Tuesday, 24 January 2023 05:54
>>>>> To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
>>>>> <sriram.yagnaraman@est.tech>
>>>>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
>>>>> <mst@redhat.com>; Marcel Apfelbaum
>> <marcel.apfelbaum@gmail.com>;
>>>> Alex
>>>>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
>>>>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
>>>> Santos
>>>>> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
>>>>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
>>>>> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov
>>>>> <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan Hajnoczi
>>>>> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
>>>> Qiuhao
>>>>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
>>>>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
>>>>> <yvugenfi@redhat.com>; Yuri Benditovich
>>>>> <yuri.benditovich@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.odaki@daynix.com> wrote:
>>>>>>>
>>>>>>> Based-on: <20230114035919.35251-1-akihiko.odaki@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.

> 
>>
>> 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.

> 
>>
>> 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
>>>>>>>
>>>>>>

RE: [PATCH v2 00/13] Introduce igb
Posted by Sriram Yagnaraman 1 year, 1 month ago

> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Monday, 30 January 2023 15:39
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
> <jasowang@redhat.com>
> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos Santos
> Moschetta <wainersm@redhat.com>; Beraldo Leal <bleal@redhat.com>;
> Cleber Rosa <crosa@redhat.com>; Laurent Vivier <lvivier@redhat.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Alexander Bulekov <alxndr@bu.edu>;
> Bandan Das <bsd@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Darren Kenny <darren.kenny@oracle.com>; Qiuhao Li
> <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com>
> >> Sent: Thursday, 26 January 2023 12:32
> >> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>; Jason Wang
> >> <jasowang@redhat.com>
> >> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> >> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Alex
> >> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> >> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> >> Santos Moschetta <wainersm@redhat.com>; Beraldo Leal
> >> <bleal@redhat.com>; Cleber Rosa <crosa@redhat.com>; Laurent Vivier
> >> <lvivier@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Alexander
> >> Bulekov <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>; Stefan
> Hajnoczi
> >> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
> Qiuhao
> >> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> >> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> >> <yvugenfi@redhat.com>; Yuri Benditovich <yuri.benditovich@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.odaki@daynix.com>; Jason Wang
> >>>> <jasowang@redhat.com>
> >>>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S. Tsirkin
> >>>> <mst@redhat.com>; Marcel Apfelbaum
> <marcel.apfelbaum@gmail.com>;
> >> Alex
> >>>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> >>>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> >>>> Santos Moschetta <wainersm@redhat.com>; Beraldo Leal
> >>>> <bleal@redhat.com>; Cleber Rosa <crosa@redhat.com>; Laurent Vivier
> >>>> <lvivier@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> >>>> Alexander Bulekov <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>;
> >>>> Stefan
> >> Hajnoczi
> >>>> <stefanha@redhat.com>; Darren Kenny <darren.kenny@oracle.com>;
> >> Qiuhao
> >>>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> >>>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> >>>> <yvugenfi@redhat.com>; Yuri Benditovich
> >>>> <yuri.benditovich@daynix.com>
> >>>> Subject: RE: [PATCH v2 00/13] Introduce igb
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> >>>>> Sent: Tuesday, 24 January 2023 05:54
> >>>>> To: Jason Wang <jasowang@redhat.com>; Sriram Yagnaraman
> >>>>> <sriram.yagnaraman@est.tech>
> >>>>> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>; Michael S.
> >>>>> Tsirkin <mst@redhat.com>; Marcel Apfelbaum
> >> <marcel.apfelbaum@gmail.com>;
> >>>> Alex
> >>>>> Bennée <alex.bennee@linaro.org>; Philippe Mathieu-Daudé
> >>>>> <philmd@linaro.org>; Thomas Huth <thuth@redhat.com>; Wainer dos
> >>>> Santos
> >>>>> Moschetta <wainersm@redhat.com>; Beraldo Leal
> <bleal@redhat.com>;
> >>>>> Cleber Rosa <crosa@redhat.com>; Laurent Vivier
> >>>>> <lvivier@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> >>>>> Alexander Bulekov <alxndr@bu.edu>; Bandan Das <bsd@redhat.com>;
> >>>>> Stefan Hajnoczi <stefanha@redhat.com>; Darren Kenny
> >>>>> <darren.kenny@oracle.com>;
> >>>> Qiuhao
> >>>>> Li <Qiuhao.Li@outlook.com>; qemu-devel@nongnu.org; qemu-
> >>>>> ppc@nongnu.org; devel@daynix.com; Yan Vugenfirer
> >>>>> <yvugenfi@redhat.com>; Yuri Benditovich
> >>>>> <yuri.benditovich@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.odaki@daynix.com> wrote:
> >>>>>>>
> >>>>>>> Based-on: <20230114035919.35251-1-akihiko.odaki@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
> >>>>>>>
> >>>>>>