[RFC,0/7] 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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_s
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.

Details on each patch of the series:
Patch 1: eal: interrupt handle API prototypes
This patch provides prototypes of all the new get set APIs, and
also rearranges the headers related to interrupt framework. Epoll
related definitions prototypes are moved into a new header i.e.
rte_epoll.h and APIs defined in rte_eal_interrupts.h which were
driver specific are moved to rte_interrupts.h (as anyways it was
accessible and used outside DPDK library. Later in the series
rte_eal_interrupts.h is removed.

Patch 2: eal/interrupts: implement get set APIs
Implementing all get, set and alloc APIs. Alloc APIs are implemented
to allocate memory for interrupt handle instance. Currently most of
the drivers defines interrupt handle instance as static but now it cant
be static as size of rte_intr_handle is unknown to all the drivers.
Drivers are expected to allocate interrupt instances during initialization
and free these instances during cleanup phase.

Patch 3: eal/interrupts: avoid direct access to interrupt handle
Modifying the interrupt framework for linux and freebsd to use these
get set alloc APIs as per requirement and avoid accessing the fields
directly.

Patch 4: test/interrupt: apply get set interrupt handle APIs
Updating interrupt test suite to use interrupt handle APIs.

Patch 5: drivers: remove direct access to interrupt handle fields
Modifying all the drivers and libraries which are currently directly
accessing the interrupt handle fields. Drivers are expected to
allocated the interrupt instance, use get set APIs with the allocated
interrupt handle and free it on cleanup.

Patch 6: eal/interrupts: make interrupt handle structure opaque
In this patch rte_eal_interrupt.h is removed, struct rte_intr_handle
definition is moved to c file to make it completely opaque. As part of
interrupt handle allocation, array like efds and elist(which are currently
static) are dynamically allocated with default size
(RTE_MAX_RXTX_INTR_VEC_ID). Later these arrays can be reallocated as per
device requirement using new API rte_intr_handle_event_list_update().
Eg, on PCI device probing MSIX size can be queried and these arrays can
be reallocated accordingly.

Patch 7: eal/alarm: introduce alarm fini routine
Introducing alarm fini routine, as the memory allocated for alarm interrupt
instance can be freed in alarm fini.

Testing performed:
1. Validated the series by running interrupts and alarm test suite.
2. Validate l3fwd power functionality with octeontx2 and i40e intel cards,
   where interrupts are expected on packet arrival.

Harman Kalra (7):
  eal: interrupt handle API prototypes
  eal/interrupts: implement get set APIs
  eal/interrupts: avoid direct access to interrupt handle
  test/interrupt: apply get set interrupt handle APIs
  drivers: remove direct access to interrupt handle fields
  eal/interrupts: make interrupt handle structure opaque
  eal/alarm: introduce alarm fini routine

 MAINTAINERS                                   |   1 +
 app/test/test_interrupts.c                    | 237 +++---
 drivers/baseband/acc100/rte_acc100_pmd.c      |  18 +-
 .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |  13 +-
 drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |  14 +-
 drivers/bus/auxiliary/auxiliary_common.c      |   2 +
 drivers/bus/auxiliary/linux/auxiliary.c       |  11 +
 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                 |  17 +-
 drivers/bus/fslmc/fslmc_vfio.c                |  32 +-
 drivers/bus/fslmc/portal/dpaa2_hw_dpio.c      |  21 +-
 drivers/bus/fslmc/portal/dpaa2_hw_pvt.h       |   2 +-
 drivers/bus/fslmc/rte_fslmc.h                 |   2 +-
 drivers/bus/ifpga/ifpga_bus.c                 |  16 +-
 drivers/bus/ifpga/rte_bus_ifpga.h             |   2 +-
 drivers/bus/pci/linux/pci.c                   |   4 +-
 drivers/bus/pci/linux/pci_uio.c               |  73 +-
 drivers/bus/pci/linux/pci_vfio.c              | 115 ++-
 drivers/bus/pci/pci_common.c                  |  29 +-
 drivers/bus/pci/pci_common_uio.c              |  21 +-
 drivers/bus/pci/rte_bus_pci.h                 |   4 +-
 drivers/bus/vmbus/linux/vmbus_bus.c           |   7 +
 drivers/bus/vmbus/linux/vmbus_uio.c           |  37 +-
 drivers/bus/vmbus/rte_bus_vmbus.h             |   2 +-
 drivers/bus/vmbus/vmbus_common_uio.c          |  24 +-
 drivers/common/cnxk/roc_cpt.c                 |   8 +-
 drivers/common/cnxk/roc_dev.c                 |  14 +-
 drivers/common/cnxk/roc_irq.c                 | 106 +--
 drivers/common/cnxk/roc_nix_irq.c             |  37 +-
 drivers/common/cnxk/roc_npa.c                 |   2 +-
 drivers/common/cnxk/roc_platform.h            |  34 +
 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             |  22 +-
 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                |  32 +-
 drivers/net/bnxt/bnxt_irq.c                   |   4 +-
 drivers/net/dpaa/dpaa_ethdev.c                |  47 +-
 drivers/net/dpaa2/dpaa2_ethdev.c              |  10 +-
 drivers/net/e1000/em_ethdev.c                 |  24 +-
 drivers/net/e1000/igb_ethdev.c                |  84 ++-
 drivers/net/ena/ena_ethdev.c                  |  36 +-
 drivers/net/enic/enic_main.c                  |  27 +-
 drivers/net/failsafe/failsafe.c               |  24 +-
 drivers/net/failsafe/failsafe_intr.c          |  45 +-
 drivers/net/failsafe/failsafe_ops.c           |  23 +-
 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                |  50 +-
 drivers/net/hns3/hns3_ethdev_vf.c             |  57 +-
 drivers/net/hns3/hns3_rxtx.c                  |   2 +-
 drivers/net/i40e/i40e_ethdev.c                |  55 +-
 drivers/net/i40e/i40e_ethdev_vf.c             |  43 +-
 drivers/net/iavf/iavf_ethdev.c                |  41 +-
 drivers/net/iavf/iavf_vchnl.c                 |   4 +-
 drivers/net/ice/ice_dcf.c                     |  10 +-
 drivers/net/ice/ice_dcf_ethdev.c              |  23 +-
 drivers/net/ice/ice_ethdev.c                  |  51 +-
 drivers/net/igc/igc_ethdev.c                  |  47 +-
 drivers/net/ionic/ionic_ethdev.c              |  12 +-
 drivers/net/ixgbe/ixgbe_ethdev.c              |  70 +-
 drivers/net/memif/memif_socket.c              |  99 ++-
 drivers/net/memif/memif_socket.h              |   4 +-
 drivers/net/memif/rte_eth_memif.c             |  63 +-
 drivers/net/memif/rte_eth_memif.h             |   2 +-
 drivers/net/mlx4/mlx4.c                       |  20 +-
 drivers/net/mlx4/mlx4.h                       |   2 +-
 drivers/net/mlx4/mlx4_intr.c                  |  48 +-
 drivers/net/mlx5/linux/mlx5_os.c              |  56 +-
 drivers/net/mlx5/linux/mlx5_socket.c          |  26 +-
 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                  |  27 +-
 drivers/net/netvsc/hn_ethdev.c                |   4 +-
 drivers/net/nfp/nfp_net.c                     |  42 +-
 drivers/net/ngbe/ngbe_ethdev.c                |  31 +-
 drivers/net/octeontx2/otx2_ethdev_irq.c       |  35 +-
 drivers/net/qede/qede_ethdev.c                |  16 +-
 drivers/net/sfc/sfc_intr.c                    |  29 +-
 drivers/net/tap/rte_eth_tap.c                 |  37 +-
 drivers/net/tap/rte_eth_tap.h                 |   2 +-
 drivers/net/tap/tap_intr.c                    |  33 +-
 drivers/net/thunderx/nicvf_ethdev.c           |  13 +
 drivers/net/thunderx/nicvf_struct.h           |   2 +-
 drivers/net/txgbe/txgbe_ethdev.c              |  36 +-
 drivers/net/txgbe/txgbe_ethdev_vf.c           |  35 +-
 drivers/net/vhost/rte_eth_vhost.c             |  78 +-
 drivers/net/virtio/virtio_ethdev.c            |  17 +-
 .../net/virtio/virtio_user/virtio_user_dev.c  |  53 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c          |  45 +-
 drivers/raw/ifpga/ifpga_rawdev.c              |  42 +-
 drivers/raw/ntb/ntb.c                         |  10 +-
 .../regex/octeontx2/otx2_regexdev_hw_access.c |   4 +-
 drivers/vdpa/ifc/ifcvf_vdpa.c                 |   5 +-
 drivers/vdpa/mlx5/mlx5_vdpa.c                 |  11 +
 drivers/vdpa/mlx5/mlx5_vdpa.h                 |   4 +-
 drivers/vdpa/mlx5/mlx5_vdpa_event.c           |  22 +-
 drivers/vdpa/mlx5/mlx5_vdpa_virtq.c           |  46 +-
 lib/bbdev/rte_bbdev.c                         |   4 +-
 lib/eal/common/eal_common_interrupts.c        | 668 +++++++++++++++++
 lib/eal/common/eal_private.h                  |  11 +
 lib/eal/common/meson.build                    |   2 +
 lib/eal/freebsd/eal.c                         |   1 +
 lib/eal/freebsd/eal_alarm.c                   |  57 +-
 lib/eal/freebsd/eal_interrupts.c              |  93 ++-
 lib/eal/include/meson.build                   |   2 +-
 lib/eal/include/rte_eal_interrupts.h          | 269 -------
 lib/eal/include/rte_eal_trace.h               |  24 +-
 lib/eal/include/rte_epoll.h                   | 117 +++
 lib/eal/include/rte_interrupts.h              | 673 +++++++++++++++++-
 lib/eal/linux/eal.c                           |   1 +
 lib/eal/linux/eal_alarm.c                     |  39 +-
 lib/eal/linux/eal_dev.c                       |  65 +-
 lib/eal/linux/eal_interrupts.c                | 294 +++++---
 lib/eal/version.map                           |  30 +
 lib/ethdev/ethdev_pci.h                       |   2 +-
 lib/ethdev/rte_ethdev.c                       |  14 +-
 129 files changed, 3763 insertions(+), 1672 deletions(-)
 create mode 100644 lib/eal/common/eal_common_interrupts.c
 delete mode 100644 lib/eal/include/rte_eal_interrupts.h
 create mode 100644 lib/eal/include/rte_epoll.h
  

On Thu, 26 Aug 2021 20:27:19 +0530
Harman Kalra <hkalra@marvell.com> wrote:

> 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://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_s
> 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.

I agree rte_intr_handle and eth_devices structure needs to be hidden.
But there does not appear to be an API to check if device supports
receive interrupt mode.

There is:
   RTE_ETH_DEV_INTR_LSC  - link state
   RTE_ETH_DEV_INTR_RMV  - interrupt on removal

but no
   RTE_ETH_DEV_INTR_RXQ  - device supports rxq interrupt

There should be a new flag reported by devices, and the intr_conf should
be checked in rte_eth_dev_configure

Doing this would require fixes many drivers and there is risk of exposing
existing sematic bugs in applications.


code
  

Hi Stephen

Thanks for your suggestion on RTE_ETH_DEV_INTR_RXQ .
Please see my comments inline.

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, October 5, 2021 9:38 PM
> To: Harman Kalra <hkalra@marvell.com>
> Cc: dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [RFC 0/7] make rte_intr_handle internal
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, 26 Aug 2021 20:27:19 +0530
> Harman Kalra <hkalra@marvell.com> wrote:
> 
> > 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.
> 
> I agree rte_intr_handle and eth_devices structure needs to be hidden.
> But there does not appear to be an API to check if device supports receive
> interrupt mode.
> 
> There is:
>    RTE_ETH_DEV_INTR_LSC  - link state
>    RTE_ETH_DEV_INTR_RMV  - interrupt on removal
> 
> but no
>    RTE_ETH_DEV_INTR_RXQ  - device supports rxq interrupt
> 
> There should be a new flag reported by devices, and the intr_conf should be
> checked in rte_eth_dev_configure
> 
> Doing this would require fixes many drivers and there is risk of exposing
> existing sematic bugs in applications.
> 
> 

Yes, currently "intr_conf.rxq" is checked by respective drivers which supports queue interrupts and enable them if set.
But driver doesn't expose if they are capable of supporting rxq interrupts, just like LSC and RMV.

RTE_ETH_DEV_INTR_RXQ   should be introduces and set by capable drivers in " dev_info.dev_flags" and applications
like l3fwd-power which sets " intr_conf.rxq" should check this flag.

I will add this enhancement to my TODO list and will send it as a new series.

Thanks
Harman

> code
>