Re: [dpdk-dev] [PATCH v8 0/9] make rte_intr_handle internal

[Raslan Darawsheh <rasland@nvidia.com>]

Hi,
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, October 25, 2021 5:27 PM
> To: hkalra@marvell.com; dev@dpdk.org
> Cc: dmitry.kozliuk@gmail.com; Raslan Darawsheh <rasland@nvidia.com>;
> NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
> Subject: [PATCH v8 0/9] make rte_intr_handle internal
> 
> Moving struct rte_intr_handle as an internal structure to avoid any ABI
> breakages in future. Since this structure defines some static arrays and
> changing respective macros breaks the ABI.
> Eg:
> Currently RTE_MAX_RXTX_INTR_VEC_ID imposes a limit of maximum 512
> MSI-X interrupts that can be defined for a PCI device, while PCI specification
> allows maximum 2048 MSI-X interrupts that can be used.
> If some PCI device requires more than 512 vectors, either change the
> RTE_MAX_RXTX_INTR_VEC_ID limit or dynamically allocate based on PCI
> device MSI-X size on probe time. Either way its an ABI breakage.
> 
> Change already included in 21.11 ABI improvement spreadsheet (item 42):
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> efense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> 3A__docs.google.com_s&amp;data=04%7C01%7Crasland%40nvidia.com%7C
> c626e0d058714bc3075a08d997c39557%7C43083d15727340c1b7db39efd9ccc17
> a%7C0%7C0%7C637707688554493769%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000&amp;sdata=y7vFUXbUzh6ise1zn8bzbfuUGv6L24gCNcUsuWKqRBk%3D&
> amp;reserved=0
> preadsheets_d_1betlC000ua5SsSiJIcC54mCCCJnW6voH5Dqv9UxeyfE_edit-
> 23gid-
> 3D0&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=5ESHPj7V-
> 7JdkxT_Z_SU6RrS37ys4U
> XudBQ_rrS5LRo&m=7dl3OmXU7QHMmWYB6V1hYJtq1cUkjfhXUwze2Si_48c
> &s=lh6DEGhR
> Bg1shODpAy3RQk-H-0uQx5icRfUBf9dtCp4&e=
> 
> This series makes struct rte_intr_handle totally opaque to the outside world
> by wrapping it inside a .c file and providing get set wrapper APIs to read or
> manipulate its fields.. Any changes to be made to any of the fields should be
> done via these get set APIs.
> Introduced a new eal_common_interrupts.c where all these APIs are
> defined and also hides struct rte_intr_handle definition.
> 
> v1:
> * Fixed freebsd compilation failure
> * Fixed seg fault in case of memif
> 
> v2:
> * Merged the prototype and implementation patch to 1.
> * Restricting allocation of single interrupt instance.
> * Removed base APIs, as they were exposing internally allocated memory
> information.
> * Fixed some memory leak issues.
> * Marked some library specific APIs as internal.
> 
> v3:
> * Removed flag from instance alloc API, rather auto detect if memory should
> be allocated using glibc malloc APIs or
> rte_malloc*
> * Added APIs for get/set windows handle.
> * Defined macros for repeated checks.
> 
> v4:
> * Rectified some typo in the APIs documentation.
> * Better names for some internal variables.
> 
> v5:
> * Reverted back to passing flag to instance alloc API, as with auto detect
> some multiprocess issues existing in the library were causing tests failure.
> * Rebased to top of tree.
> 
> v6:
> * renamed RTE_INTR_INSTANCE_F_UNSHARED as
> RTE_INTR_INSTANCE_F_PRIVATE,
> * changed API and removed need for alloc_flag content exposure
>   (see rte_intr_instance_dup() in patch 1 and 2),
> * exported all symbols for Windows,
> * fixed leak in unit tests in case of alloc failure,
> * split (previously) patch 4 into three patches
>   * (now) patch 4 only concerns alarm and (previously) patch 6 cleanup bits
>     are squashed in it,
>   * (now) patch 5 concerns other libraries updates,
>   * (now) patch 6 concerns drivers updates:
>     * instance allocation is moved to probing for auxiliary,
>     * there might be a bug for PCI drivers non requesting
>       RTE_PCI_DRV_NEED_MAPPING, but code is left as v5,
> * split (previously) patch 5 into three patches
>   * (now) patch 7 only hides structure, but keep it in a EAL private
>     header, this makes it possible to keep info in tracepoints,
>   * (now) patch 8 deals with VFIO/UIO internal fds merge,
>   * (now) patch 9 extends event list,
> 
> v7:
> * fixed compilation on FreeBSD,
> * removed unused interrupt handle in FreeBSD alarm code,
> * fixed interrupt handle allocation for PCI drivers without
>   RTE_PCI_DRV_NEED_MAPPING,
> 
> v8:
> * lowered logs level to DEBUG in sanity checks,
> * fixed corner case with vector list access,
> 
> --
> David Marchand
> 
> Harman Kalra (9):
>   interrupts: add allocator and accessors
>   interrupts: remove direct access to interrupt handle
>   test/interrupts: remove direct access to interrupt handle
>   alarm: remove direct access to interrupt handle
>   lib: remove direct access to interrupt handle
>   drivers: remove direct access to interrupt handle
>   interrupts: make interrupt handle structure opaque
>   interrupts: rename device specific file descriptor
>   interrupts: extend event list
> 
>  MAINTAINERS                                   |   1 +
>  app/test/test_interrupts.c                    | 164 +++--
>  drivers/baseband/acc100/rte_acc100_pmd.c      |  14 +-
>  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |  24 +-
>  drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |  24 +-
>  drivers/bus/auxiliary/auxiliary_common.c      |  17 +-
>  drivers/bus/auxiliary/rte_bus_auxiliary.h     |   2 +-
>  drivers/bus/dpaa/dpaa_bus.c                   |  28 +-
>  drivers/bus/dpaa/rte_dpaa_bus.h               |   2 +-
>  drivers/bus/fslmc/fslmc_bus.c                 |  14 +-
>  drivers/bus/fslmc/fslmc_vfio.c                |  30 +-
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c      |  18 +-
>  drivers/bus/fslmc/portal/dpaa2_hw_pvt.h       |   2 +-
>  drivers/bus/fslmc/rte_fslmc.h                 |   2 +-
>  drivers/bus/ifpga/ifpga_bus.c                 |  13 +-
>  drivers/bus/ifpga/rte_bus_ifpga.h             |   2 +-
>  drivers/bus/pci/bsd/pci.c                     |  20 +-
>  drivers/bus/pci/linux/pci.c                   |   4 +-
>  drivers/bus/pci/linux/pci_uio.c               |  69 +-
>  drivers/bus/pci/linux/pci_vfio.c              | 108 ++-
>  drivers/bus/pci/pci_common.c                  |  47 +-
>  drivers/bus/pci/pci_common_uio.c              |  21 +-
>  drivers/bus/pci/rte_bus_pci.h                 |   4 +-
>  drivers/bus/vmbus/linux/vmbus_bus.c           |   6 +
>  drivers/bus/vmbus/linux/vmbus_uio.c           |  35 +-
>  drivers/bus/vmbus/rte_bus_vmbus.h             |   2 +-
>  drivers/bus/vmbus/vmbus_common_uio.c          |  23 +-
>  drivers/common/cnxk/roc_cpt.c                 |   8 +-
>  drivers/common/cnxk/roc_dev.c                 |  14 +-
>  drivers/common/cnxk/roc_irq.c                 | 107 +--
>  drivers/common/cnxk/roc_nix_inl_dev_irq.c     |   8 +-
>  drivers/common/cnxk/roc_nix_irq.c             |  36 +-
>  drivers/common/cnxk/roc_npa.c                 |   2 +-
>  drivers/common/cnxk/roc_platform.h            |  49 +-
>  drivers/common/cnxk/roc_sso.c                 |   4 +-
>  drivers/common/cnxk/roc_tim.c                 |   4 +-
>  drivers/common/octeontx2/otx2_dev.c           |  14 +-
>  drivers/common/octeontx2/otx2_irq.c           | 117 ++--
>  .../octeontx2/otx2_cryptodev_hw_access.c      |   4 +-
>  drivers/event/octeontx2/otx2_evdev_irq.c      |  12 +-
>  drivers/mempool/octeontx2/otx2_mempool.c      |   2 +-
>  drivers/net/atlantic/atl_ethdev.c             |  20 +-
>  drivers/net/avp/avp_ethdev.c                  |   8 +-
>  drivers/net/axgbe/axgbe_ethdev.c              |  12 +-
>  drivers/net/axgbe/axgbe_mdio.c                |   6 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c              |  10 +-
>  drivers/net/bnxt/bnxt_ethdev.c                |  33 +-
>  drivers/net/bnxt/bnxt_irq.c                   |   4 +-
>  drivers/net/dpaa/dpaa_ethdev.c                |  48 +-
>  drivers/net/dpaa2/dpaa2_ethdev.c              |  10 +-
>  drivers/net/e1000/em_ethdev.c                 |  23 +-
>  drivers/net/e1000/igb_ethdev.c                |  79 +--
>  drivers/net/ena/ena_ethdev.c                  |  35 +-
>  drivers/net/enic/enic_main.c                  |  26 +-
>  drivers/net/failsafe/failsafe.c               |  21 +-
>  drivers/net/failsafe/failsafe_intr.c          |  43 +-
>  drivers/net/failsafe/failsafe_ops.c           |  19 +-
>  drivers/net/failsafe/failsafe_private.h       |   2 +-
>  drivers/net/fm10k/fm10k_ethdev.c              |  32 +-
>  drivers/net/hinic/hinic_pmd_ethdev.c          |  10 +-
>  drivers/net/hns3/hns3_ethdev.c                |  57 +-
>  drivers/net/hns3/hns3_ethdev_vf.c             |  64 +-
>  drivers/net/hns3/hns3_rxtx.c                  |   2 +-
>  drivers/net/i40e/i40e_ethdev.c                |  53 +-
>  drivers/net/iavf/iavf_ethdev.c                |  42 +-
>  drivers/net/iavf/iavf_vchnl.c                 |   4 +-
>  drivers/net/ice/ice_dcf.c                     |  10 +-
>  drivers/net/ice/ice_dcf_ethdev.c              |  21 +-
>  drivers/net/ice/ice_ethdev.c                  |  49 +-
>  drivers/net/igc/igc_ethdev.c                  |  45 +-
>  drivers/net/ionic/ionic_ethdev.c              |  17 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c              |  66 +-
>  drivers/net/memif/memif_socket.c              | 108 ++-
>  drivers/net/memif/memif_socket.h              |   4 +-
>  drivers/net/memif/rte_eth_memif.c             |  56 +-
>  drivers/net/memif/rte_eth_memif.h             |   2 +-
>  drivers/net/mlx4/mlx4.c                       |  19 +-
>  drivers/net/mlx4/mlx4.h                       |   2 +-
>  drivers/net/mlx4/mlx4_intr.c                  |  47 +-
>  drivers/net/mlx5/linux/mlx5_os.c              |  55 +-
>  drivers/net/mlx5/linux/mlx5_socket.c          |  25 +-
>  drivers/net/mlx5/mlx5.h                       |   6 +-
>  drivers/net/mlx5/mlx5_rxq.c                   |  43 +-
>  drivers/net/mlx5/mlx5_trigger.c               |   4 +-
>  drivers/net/mlx5/mlx5_txpp.c                  |  25 +-
>  drivers/net/netvsc/hn_ethdev.c                |   4 +-
>  drivers/net/nfp/nfp_common.c                  |  34 +-
>  drivers/net/nfp/nfp_ethdev.c                  |  13 +-
>  drivers/net/nfp/nfp_ethdev_vf.c               |  13 +-
>  drivers/net/ngbe/ngbe_ethdev.c                |  29 +-
>  drivers/net/octeontx2/otx2_ethdev_irq.c       |  35 +-
>  drivers/net/qede/qede_ethdev.c                |  16 +-
>  drivers/net/sfc/sfc_intr.c                    |  30 +-
>  drivers/net/tap/rte_eth_tap.c                 |  33 +-
>  drivers/net/tap/rte_eth_tap.h                 |   2 +-
>  drivers/net/tap/tap_intr.c                    |  33 +-
>  drivers/net/thunderx/nicvf_ethdev.c           |  10 +
>  drivers/net/thunderx/nicvf_struct.h           |   2 +-
>  drivers/net/txgbe/txgbe_ethdev.c              |  38 +-
>  drivers/net/txgbe/txgbe_ethdev_vf.c           |  33 +-
>  drivers/net/vhost/rte_eth_vhost.c             |  80 ++-
>  drivers/net/virtio/virtio_ethdev.c            |  21 +-
>  .../net/virtio/virtio_user/virtio_user_dev.c  |  56 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c          |  43 +-
>  drivers/raw/ifpga/ifpga_rawdev.c              |  62 +-
>  drivers/raw/ntb/ntb.c                         |   9 +-
>  .../regex/octeontx2/otx2_regexdev_hw_access.c |   4 +-
>  drivers/vdpa/ifc/ifcvf_vdpa.c                 |   5 +-
>  drivers/vdpa/mlx5/mlx5_vdpa.c                 |   8 +
>  drivers/vdpa/mlx5/mlx5_vdpa.h                 |   4 +-
>  drivers/vdpa/mlx5/mlx5_vdpa_event.c           |  21 +-
>  drivers/vdpa/mlx5/mlx5_vdpa_virtq.c           |  44 +-
>  lib/bbdev/rte_bbdev.c                         |   4 +-
>  lib/eal/common/eal_common_interrupts.c        | 500 ++++++++++++++
>  lib/eal/common/eal_interrupts.h               |  30 +
>  lib/eal/common/eal_private.h                  |  10 +
>  lib/eal/common/meson.build                    |   1 +
>  lib/eal/freebsd/eal.c                         |   1 +
>  lib/eal/freebsd/eal_alarm.c                   |  35 +-
>  lib/eal/freebsd/eal_interrupts.c              |  85 ++-
>  lib/eal/include/meson.build                   |   2 +-
>  lib/eal/include/rte_eal_interrupts.h          | 269 --------
>  lib/eal/include/rte_eal_trace.h               |  10 +-
>  lib/eal/include/rte_epoll.h                   | 118 ++++
>  lib/eal/include/rte_interrupts.h              | 651 +++++++++++++++++-
>  lib/eal/linux/eal.c                           |   1 +
>  lib/eal/linux/eal_alarm.c                     |  32 +-
>  lib/eal/linux/eal_dev.c                       |  57 +-
>  lib/eal/linux/eal_interrupts.c                | 304 ++++----
>  lib/eal/version.map                           |  45 +-
>  lib/ethdev/ethdev_pci.h                       |   2 +-
>  lib/ethdev/rte_ethdev.c                       |  14 +-
>  132 files changed, 3449 insertions(+), 1748 deletions(-)  create mode 100644
> lib/eal/common/eal_common_interrupts.c
>  create mode 100644 lib/eal/common/eal_interrupts.h  delete mode 100644
> lib/eal/include/rte_eal_interrupts.h
>  create mode 100644 lib/eal/include/rte_epoll.h
> 
> --
> 2.23.0

Tested-by: Raslan Darawsheh <rasland@nvidia.com>

Kindest regards,
Raslan Darawsheh