2021-10-23 02:19 (UTC+0530), Harman Kalra:
> Prototype/Implement get set APIs for interrupt handle fields.
> User wont be able to access any of the interrupt handle fields
> directly while should use these get/set APIs to access/manipulate
> them.
>
> Internal interrupt header i.e. rte_eal_interrupt.h is rearranged,
> as APIs defined are moved to rte_interrupts.h and epoll specific
> definitions are moved to a new header rte_epoll.h.
> Later in the series rte_eal_interrupt.h will be removed.
>
> Signed-off-by: Harman Kalra <hka...@marvell.com>
> Acked-by: Ray Kinsella <m...@ashroe.eu>
Hi Harman,
After fixing some comment below feel free to add:
Acked-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>
> ---
> MAINTAINERS | 1 +
> lib/eal/common/eal_common_interrupts.c | 421 ++++++++++++++++
> lib/eal/common/meson.build | 1 +
> lib/eal/include/meson.build | 1 +
> lib/eal/include/rte_eal_interrupts.h | 209 +-------
> lib/eal/include/rte_epoll.h | 118 +++++
> lib/eal/include/rte_interrupts.h | 648 ++++++++++++++++++++++++-
> lib/eal/version.map | 46 +-
> 8 files changed, 1232 insertions(+), 213 deletions(-)
> create mode 100644 lib/eal/common/eal_common_interrupts.c
> create mode 100644 lib/eal/include/rte_epoll.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04ea23a04a..d2950400d2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -211,6 +211,7 @@ F: app/test/test_memzone.c
>
> Interrupt Subsystem
> M: Harman Kalra <hka...@marvell.com>
> +F: lib/eal/include/rte_epoll.h
> F: lib/eal/*/*interrupts.*
> F: app/test/test_interrupts.c
>
> diff --git a/lib/eal/common/eal_common_interrupts.c
> b/lib/eal/common/eal_common_interrupts.c
> [...]
> +int rte_intr_efds_index_get(const struct rte_intr_handle *intr_handle,
> + int index)
> +{
> + CHECK_VALID_INTR_HANDLE(intr_handle);
> +
> + if (index >= intr_handle->nb_intr) {
> + RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", index,
"size" -> "index"
> + intr_handle->nb_intr);
> + rte_errno = EINVAL;
> + goto fail;
> + }
> +
> + return intr_handle->efds[index];
> +fail:
> + return -rte_errno;
> +}
> [...]
> +int rte_intr_vec_list_alloc(struct rte_intr_handle *intr_handle,
> + const char *name, int size)
> +{
> + CHECK_VALID_INTR_HANDLE(intr_handle);
> +
> + /* Vector list already allocated */
> + if (intr_handle->intr_vec != NULL)
> + return 0;
What if `size > intr_handle->vec_list_size`?
> +
> + if (size > intr_handle->nb_intr) {
> + RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", size,
> + intr_handle->nb_intr);
> + rte_errno = ERANGE;
> + goto fail;
> + }
> +
> + intr_handle->intr_vec = rte_zmalloc(name, size * sizeof(int), 0);
> + if (intr_handle->intr_vec == NULL) {
> + RTE_LOG(ERR, EAL, "Failed to allocate %d intr_vec", size);
> + rte_errno = ENOMEM;
> + goto fail;
> + }
> +
> + intr_handle->vec_list_size = size;
> +
> + return 0;
> +fail:
> + return -rte_errno;
> +}
> [...]
> diff --git a/lib/eal/include/rte_interrupts.h
> b/lib/eal/include/rte_interrupts.h
> index cc3bf45d8c..a29232e16a 100644
> --- a/lib/eal/include/rte_interrupts.h
> +++ b/lib/eal/include/rte_interrupts.h
> @@ -5,8 +5,11 @@
> #ifndef _RTE_INTERRUPTS_H_
> #define _RTE_INTERRUPTS_H_
>
> +#include <stdbool.h>
> +
> #include <rte_common.h>
> #include <rte_compat.h>
> +#include <rte_epoll.h>
>
> /**
> * @file
> @@ -22,6 +25,16 @@ extern "C" {
> /** Interrupt handle */
> struct rte_intr_handle;
>
> +/** Interrupt instance allocation flags
> + * @see rte_intr_instance_alloc
> + */
> +/** Interrupt instance would not be shared within primary secondary process.
> */
> +#define RTE_INTR_INSTANCE_F_UNSHARED 0x00000001
> +/** Interrupt instance could be shared within primary secondary process. */
> +#define RTE_INTR_INSTANCE_F_SHARED 0x00000002
Nits:
"would not" -> "will not"
"could" -> "will be"
"within primary secondary process" ->
"between primary and secondary processes"
You previously suggested PRIVATE instead of UNSHARED,
it sounded better, but no strog opinion.
> [...]
> +/**
> + * @internal
> + * This API is used to populate interrupt handle with src handler fields.
"handler" -> "handle"
> + *
> + * @param intr_handle
> + * Interrupt handle pointer.
> + * @param src
> + * Source interrupt handle to be cloned.
> + *
> + * @return
> + * - On success, zero.
> + * - On failure, a negative value and rte_errno is set.
> + */
> +__rte_internal
> +int
> +rte_intr_instance_copy(struct rte_intr_handle *intr_handle,
> + const struct rte_intr_handle *src);
> [...]
> +/**
> + * @internal
> + * This API returns the sources from where memory is allocated for interrupt
> + * instance.
> + *
> + * @param intr_handle
> + * pointer to the interrupt handle.
> + *
> + * @return
> + * - On success, 1 corresponds to memory allocated via DPDK allocator APIs
> + * - On success, 0 corresponds to memory allocated from traditional heap.
> + * - On failure, negative value.
> + */
> +__rte_internal
> +int
> +rte_intr_instance_alloc_flag_get(const struct rte_intr_handle *intr_handle);
It returns flags passed on allow,
so return value type and description is inaccurate.
Should be uint32_t and a reference to the flag constants.
