From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E06C6A0C45; Mon, 25 Oct 2021 15:10:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7B28740E32; Mon, 25 Oct 2021 15:10:05 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id DFF6D4003E for ; Mon, 25 Oct 2021 15:10:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635167403; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=B8LbRR1pFLbFRWx25WRh3uxlCq61Ipu81WRlU15NUJg=; b=Mb9XO3B39hbpYbTqUg5YZoCczsjkJoGNTi/LovZZLENai1cG6M3cEKDq4uWNkp0YdQVx4F SxyCh8cURPe4fRmkPdmLLB9ikNJcVonOofglN9rnWc2UBWKvlFsGCOCkLB4y4GM1//3/YH mCVgS+dVnVsExOseTsxBO9YucUZe5Wo= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-596-96f_qo3xPCu7nGO1zgNLUA-1; Mon, 25 Oct 2021 09:10:01 -0400 X-MC-Unique: 96f_qo3xPCu7nGO1zgNLUA-1 Received: by mail-lf1-f72.google.com with SMTP id p19-20020a056512139300b003ff6dfea137so5191166lfa.9 for ; Mon, 25 Oct 2021 06:10:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=B8LbRR1pFLbFRWx25WRh3uxlCq61Ipu81WRlU15NUJg=; b=3s0IBWu+lQfYEUlbdzZbLuyfgJDTc0GMi++Gs6Rfi/Q5Gp/PseXaV2qAmxphLKf3Vl /wEhA9YHWsp6LC8IUj4RNVxsTfcEoFGqYeZXdZMzQvjcHjgIHNwaB/HVuEIgZsSTrwfT fkUKE+F7zlN64wuw1K3YzsT7u+xLEwD6n5fouhNugGmkag/MVYGCFKpgDdclMJVB+jE0 5ZPQtu51bNj3iWa1Cgfyq8pKltpiqgLtITKJMGrMb21Rq8/ai23R9ru8lvfWS6A0BsNt F4DzOVspLG7sTaKg8PJvmPjW0GZvQYxycNoEBcdVRXYfv4bpEOrGk5EvL968wDL/RDjH GUXw== X-Gm-Message-State: AOAM532qrBuBlec3F/hSJHP8M0dQQ16VFT1Ii+eVnXMwewg9ewQSuelB 940mMQkb07WHofF/xibmVXgo6NWtWqLkvKrPLEUGGMXdRqYmd2SE/03MHUAhfdr//7AYRgM5kLA Oz7G4Gg31+E9euIL/NzA= X-Received: by 2002:ac2:4d56:: with SMTP id 22mr3671860lfp.265.1635167399523; Mon, 25 Oct 2021 06:09:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzRyitSYmvkrFQ3i0tmw0n1rN5xtiSlO5z8IBQC4gAeqVbXNRa9Mthe9VLT+p4CMss35o0F9f7hwIpkWFTxJD4= X-Received: by 2002:ac2:4d56:: with SMTP id 22mr3671842lfp.265.1635167399258; Mon, 25 Oct 2021 06:09:59 -0700 (PDT) MIME-Version: 1.0 References: <20210826145726.102081-1-hkalra@marvell.com> <20211022204934.132186-1-hkalra@marvell.com> In-Reply-To: From: David Marchand Date: Mon, 25 Oct 2021 15:09:47 +0200 Message-ID: To: Raslan Darawsheh Cc: Harman Kalra , "dev@dpdk.org" , "dmitry.kozliuk@gmail.com" , "mdr@ashroe.eu" , NBU-Contact-Thomas Monjalon Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v5 0/6] make rte_intr_handle internal X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Oct 25, 2021 at 3:04 PM Raslan Darawsheh wrote= : > > Hi, > > > -----Original Message----- > > From: dev On Behalf Of Harman Kalra > > Sent: Friday, October 22, 2021 11:49 PM > > To: dev@dpdk.org > > Cc: david.marchand@redhat.com; dmitry.kozliuk@gmail.com; > > mdr@ashroe.eu; NBU-Contact-Thomas Monjalon ; > > Harman Kalra > > Subject: [dpdk-dev] [PATCH v5 0/6] 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=3Dhttps%3A%2F%2Furl= d > > efense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps- > > 3A__docs.google.com_s&data=3D04%7C01%7Crasland%40nvidia.com%7C > > 567d8ee2e3c842a9e59808d9959d822e%7C43083d15727340c1b7db39efd9ccc1 > > 7a%7C0%7C0%7C637705326003996997%7CUnknown%7CTWFpbGZsb3d8eyJ > > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% > > 7C1000&sdata=3D7UgxpkEtH%2Fnjk7xo9qELjqWi58XLzzCH2pimeDWLzvc% > > 3D&reserved=3D0 > > preadsheets_d_1betlC000ua5SsSiJIcC54mCCCJnW6voH5Dqv9UxeyfE_edit- > > 23gid- > > 3D0&d=3DDwICaQ&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3D5ESHPj7V- > > 7JdkxT_Z_SU6RrS37ys4U > > XudBQ_rrS5LRo&m=3D7dl3OmXU7QHMmWYB6V1hYJtq1cUkjfhXUwze2Si_48c > > &s=3Dlh6DEGhR > > Bg1shODpAy3RQk-H-0uQx5icRfUBf9dtCp4&e=3D > > > > 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 API= s > > 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/interrupts: implement get set APIs > > This patch provides prototypes and implementation of all the new > > get set 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. > > This patch 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: 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 3: test/interrupt: apply get set interrupt handle APIs > > Updating interrupt test suite to use interrupt handle APIs. > > > > Patch 4: 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 5: 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 curren= tly > > static) are dynamically allocated with default size > > (RTE_MAX_RXTX_INTR_VEC_ID). Later these arrays can be reallocated as pe= r > > 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 6: eal/alarm: introduce alarm fini routine > > Introducing alarm fini routine, as the memory allocated for alarm inter= rupt > > 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 car= ds, > > where interrupts are expected on packet arrival. > > > > 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. > > > > Harman Kalra (6): > > 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 > > eal/interrupts: make interrupt handle structure opaque > > eal/alarm: introduce alarm fini routine > > > > MAINTAINERS | 1 + > > app/test/test_interrupts.c | 163 +++-- > > drivers/baseband/acc100/rte_acc100_pmd.c | 18 +- > > .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 21 +- > > drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 21 +- > > drivers/bus/auxiliary/auxiliary_common.c | 2 + > > drivers/bus/auxiliary/linux/auxiliary.c | 10 + > > 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 | 16 +- > > drivers/bus/fslmc/fslmc_vfio.c | 32 +- > > drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 20 +- > > drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 2 +- > > drivers/bus/fslmc/rte_fslmc.h | 2 +- > > drivers/bus/ifpga/ifpga_bus.c | 15 +- > > drivers/bus/ifpga/rte_bus_ifpga.h | 2 +- > > drivers/bus/pci/bsd/pci.c | 21 +- > > 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 | 6 + > > 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 | 108 +-- > > 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 | 47 +- > > 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 | 23 +- > > 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 | 111 ++- > > drivers/net/memif/memif_socket.h | 4 +- > > drivers/net/memif/rte_eth_memif.c | 61 +- > > 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 | 53 +- > > drivers/net/mlx5/linux/mlx5_socket.c | 25 +- > > drivers/net/mlx5/mlx5.h | 6 +- > > drivers/net/mlx5/mlx5_rxq.c | 42 +- > > drivers/net/mlx5/mlx5_trigger.c | 4 +- > > drivers/net/mlx5/mlx5_txpp.c | 26 +- > > 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 | 36 +- > > drivers/net/tap/rte_eth_tap.h | 2 +- > > drivers/net/tap/tap_intr.c | 32 +- > > drivers/net/thunderx/nicvf_ethdev.c | 12 + > > 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 | 76 +- > > drivers/net/virtio/virtio_ethdev.c | 21 +- > > .../net/virtio/virtio_user/virtio_user_dev.c | 48 +- > > 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 | 10 + > > drivers/vdpa/mlx5/mlx5_vdpa.h | 4 +- > > drivers/vdpa/mlx5/mlx5_vdpa_event.c | 22 +- > > drivers/vdpa/mlx5/mlx5_vdpa_virtq.c | 45 +- > > lib/bbdev/rte_bbdev.c | 4 +- > > lib/eal/common/eal_common_interrupts.c | 588 +++++++++++++++ > > lib/eal/common/eal_private.h | 11 + > > lib/eal/common/meson.build | 1 + > > lib/eal/freebsd/eal.c | 1 + > > lib/eal/freebsd/eal_alarm.c | 53 +- > > lib/eal/freebsd/eal_interrupts.c | 112 ++- > > 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 | 118 ++++ > > lib/eal/include/rte_interrupts.h | 668 +++++++++++++++++- > > lib/eal/linux/eal.c | 1 + > > lib/eal/linux/eal_alarm.c | 37 +- > > lib/eal/linux/eal_dev.c | 63 +- > > lib/eal/linux/eal_interrupts.c | 303 +++++--- > > lib/eal/version.map | 46 +- > > lib/ethdev/ethdev_pci.h | 2 +- > > lib/ethdev/rte_ethdev.c | 14 +- > > 132 files changed, 3631 insertions(+), 1713 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 > > > > -- > > 2.18.0 > > This series is causing this seg fault with MLX5 pmd: > Thread 1 "dpdk-l3fwd-powe" received signal SIGSEGV, Segmentation fault. > rte_intr_free_epoll_fd (intr_handle=3D0x0) at ../lib/eal/linux/eal_interr= upts.c:1512 > 1512 if (__atomic_load_n(&rev->status, > (gdb) bt > #0 rte_intr_free_epoll_fd (intr_handle=3D0x0) at ../lib/eal/linux/eal_in= terrupts.c:1512 > #1 0x0000555556de7814 in mlx5_rx_intr_vec_disable (dev=3D0x55555b554a40 = ) at ../drivers/net/mlx5/mlx5_rxq.c:934 > #2 0x0000555556de73da in mlx5_rx_intr_vec_enable (dev=3D0x55555b554a40 <= rte_eth_devices>) at ../drivers/net/mlx5/mlx5_rxq.c:836 > #3 0x0000555556e04012 in mlx5_dev_start (dev=3D0x55555b554a40 ) at ../drivers/net/mlx5/mlx5_trigger.c:1146 > #4 0x0000555555b82da7 in rte_eth_dev_start (port_id=3D0) at ../lib/ethde= v/rte_ethdev.c:1823 > #5 0x000055555575e66d in main (argc=3D7, argv=3D0x7fffffffe3f0) at ../ex= amples/l3fwd-power/main.c:2811 > (gdb) f 1 > #1 0x0000555556de7814 in mlx5_rx_intr_vec_disable (dev=3D0x55555b554a40 = ) at ../drivers/net/mlx5/mlx5_rxq.c:934 > 934 rte_intr_free_epoll_fd(intr_handle); > > > It can be easily reproduced as following: > dpdk-l3fwd-power -n 4 -a 0000:08:00.0,txq_inline_mpw=3D439,rx_vec_en=3D1 = -a 0000:08:00.,txq_inline_mpw=3D439,rx_vec_en=3D1 -c 0xfffffff -- -p 0x3 -P= --interrupt-only --parse-ptype --config=3D'(0, 0, 0)(1, 0, 1)(0, 1, 2)(1, = 1, 3)(0, 2, 4)(1, 2, 5)(0, 3, 6)(1, 3, 7)' > That confirms my suspicion on pci bus update that look at RTE_PCI_DRV_NEED_MAPPING. v7 incoming. --=20 David Marchand