Re: [PATCH v7 00/17] NBL PMD for Nebulamatrix NICs

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 28 Aug 2025 20:27:48 -0700
Dimon Zhao <dimon.zhao@nebula-matrix.com> wrote:

> Features:
> ---------
> - MTU update
> - promisc mode set
> - xstats
> - Basic stats 
> 
> Support NICs:
> -------------
> - S1205CQ-A00CHT
> - S1105AS-A00CHT
> - S1055AS-A00CHT
> - S1052AS-A00CHT
> - S1051AS-A00CHT
> - S1045XS-A00CHT
> - S1205CQ-A00CSP
> - S1055AS-A00CSP
> - S1052AS-A00CSP
> 
> Dimon Zhao (17):
>   net/nbl: add doc and minimum nbl build framework
>   net/nbl: add simple probe/remove and log module
>   net/nbl: add HW layer definitions and implementation
>   net/nbl: add Channel layer definitions and implementation
>   net/nbl: add Resource layer definitions and implementation
>   net/nbl: add Dispatch layer definitions and implementation
>   net/nbl: add Dev layer definitions and implementation
>   net/nbl: add complete device init and uninit functionality
>   net/nbl: add UIO and VFIO mode for nbl
>   net/nbl: add nbl coexistence mode for nbl
>   net/nbl: add nbl ethdev configuration
>   net/nbl: add nbl device rxtx queue setup and release ops
>   net/nbl: add nbl device start and stop ops
>   net/nbl: add nbl device Tx and Rx burst
>   net/nbl: add nbl ethdev infos get
>   net/nbl: add nbl device xstats and stats
>   net/nbl: nbl device support set MTU and promisc
> 
>  .mailmap                                      |    4 +
>  MAINTAINERS                                   |    9 +
>  doc/guides/nics/features/nbl.ini              |   31 +
>  doc/guides/nics/index.rst                     |    1 +
>  doc/guides/nics/nbl.rst                       |   87 ++
>  doc/guides/rel_notes/release_25_11.rst        |    4 +
>  drivers/net/meson.build                       |    1 +
>  drivers/net/nbl/meson.build                   |   26 +
>  drivers/net/nbl/nbl_common/nbl_common.c       |   47 +
>  drivers/net/nbl/nbl_common/nbl_common.h       |   10 +
>  drivers/net/nbl/nbl_common/nbl_userdev.c      |  761 ++++++++++
>  drivers/net/nbl/nbl_common/nbl_userdev.h      |   21 +
>  drivers/net/nbl/nbl_core.c                    |  105 ++
>  drivers/net/nbl/nbl_core.h                    |   98 ++
>  drivers/net/nbl/nbl_dev/nbl_dev.c             | 1041 +++++++++++++
>  drivers/net/nbl/nbl_dev/nbl_dev.h             |   89 ++
>  drivers/net/nbl/nbl_dispatch.c                | 1326 +++++++++++++++++
>  drivers/net/nbl/nbl_dispatch.h                |   31 +
>  drivers/net/nbl/nbl_ethdev.c                  |  141 ++
>  drivers/net/nbl/nbl_ethdev.h                  |   32 +
>  drivers/net/nbl/nbl_hw/nbl_channel.c          |  980 ++++++++++++
>  drivers/net/nbl/nbl_hw/nbl_channel.h          |  131 ++
>  drivers/net/nbl/nbl_hw/nbl_hw.h               |   28 +
>  .../nbl_hw/nbl_hw_leonis/nbl_hw_leonis_snic.c |  228 +++
>  .../nbl_hw/nbl_hw_leonis/nbl_hw_leonis_snic.h |   53 +
>  .../nbl/nbl_hw/nbl_hw_leonis/nbl_res_leonis.c |  253 ++++
>  .../nbl/nbl_hw/nbl_hw_leonis/nbl_res_leonis.h |   10 +
>  drivers/net/nbl/nbl_hw/nbl_resource.c         |    5 +
>  drivers/net/nbl/nbl_hw/nbl_resource.h         |  249 ++++
>  drivers/net/nbl/nbl_hw/nbl_txrx.c             |  904 +++++++++++
>  drivers/net/nbl/nbl_hw/nbl_txrx.h             |   40 +
>  drivers/net/nbl/nbl_hw/nbl_txrx_ops.h         |   91 ++
>  drivers/net/nbl/nbl_include/nbl_def_channel.h |  458 ++++++
>  drivers/net/nbl/nbl_include/nbl_def_common.h  |  120 ++
>  drivers/net/nbl/nbl_include/nbl_def_dev.h     |   26 +
>  .../net/nbl/nbl_include/nbl_def_dispatch.h    |   99 ++
>  drivers/net/nbl/nbl_include/nbl_def_hw.h      |   35 +
>  .../net/nbl/nbl_include/nbl_def_resource.h    |   91 ++
>  drivers/net/nbl/nbl_include/nbl_include.h     |  201 +++
>  drivers/net/nbl/nbl_include/nbl_logs.h        |   25 +
>  .../net/nbl/nbl_include/nbl_product_base.h    |   31 +
>  41 files changed, 7923 insertions(+)
>  create mode 100644 doc/guides/nics/features/nbl.ini
>  create mode 100644 doc/guides/nics/nbl.rst
>  create mode 100644 drivers/net/nbl/meson.build
>  create mode 100644 drivers/net/nbl/nbl_common/nbl_common.c
>  create mode 100644 drivers/net/nbl/nbl_common/nbl_common.h
>  create mode 100644 drivers/net/nbl/nbl_common/nbl_userdev.c
>  create mode 100644 drivers/net/nbl/nbl_common/nbl_userdev.h
>  create mode 100644 drivers/net/nbl/nbl_core.c
>  create mode 100644 drivers/net/nbl/nbl_core.h
>  create mode 100644 drivers/net/nbl/nbl_dev/nbl_dev.c
>  create mode 100644 drivers/net/nbl/nbl_dev/nbl_dev.h
>  create mode 100644 drivers/net/nbl/nbl_dispatch.c
>  create mode 100644 drivers/net/nbl/nbl_dispatch.h
>  create mode 100644 drivers/net/nbl/nbl_ethdev.c
>  create mode 100644 drivers/net/nbl/nbl_ethdev.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_channel.c
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_channel.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_hw.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_hw_leonis/
> 		    nbl_hw_leonis_snic.c
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_hw_leonis/
> 		    nbl_hw_leonis_snic.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_hw_leonis/nbl_res_leonis.c
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_hw_leonis/nbl_res_leonis.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_resource.c
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_resource.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_txrx.c
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_txrx.h
>  create mode 100644 drivers/net/nbl/nbl_hw/nbl_txrx_ops.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_channel.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_common.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_dev.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_dispatch.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_hw.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_def_resource.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_include.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_logs.h
>  create mode 100644 drivers/net/nbl/nbl_include/nbl_product_base.h
> 

Overall the driver looks good but needs to be updated.

Please address the following:
   1. release notes changes in main branch so patch doesn't apply cleanly
 
   2. don't use rte_memcpy(). The places you are using it memcpy() would
      be as fast and the compiler has more bounds checking with memcpy().

   3. probe function should not log at INFO level, keep it to DEBUG only.
      Same for nbl_mdev_map_device().
      A driver should print no messages on normal successful startup and
      when device is not present.

   4. The driver is mostly namespace clean, symbols are prefixed by nbl_.
      There is a global symbol dev_ops, which could conflict when linked
      statically with other code. Looks unused??

struct nbl_dev_ops dev_ops = {
};