Re: [dpdk-dev] [PATCH v4 3/7] eal/interrupts: avoid direct access to interrupt handle

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2021-10-20 00:05 (UTC+0530), Harman Kalra:
> Making changes to the interrupt framework to use interrupt handle
> APIs to get/set any field. Direct access to any of the fields
> should be avoided to avoid any ABI breakage in future.

I get and accept the point why EAL also should use the API.
However, mentioning ABI is still a wrong wording.
There is no ABI between EAL structures and EAL functions by definition of ABI.

> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
>  lib/eal/freebsd/eal_interrupts.c |  92 ++++++----
>  lib/eal/linux/eal_interrupts.c   | 287 +++++++++++++++++++------------
>  2 files changed, 234 insertions(+), 145 deletions(-)
> 
> diff --git a/lib/eal/freebsd/eal_interrupts.c b/lib/eal/freebsd/eal_interrupts.c
[...]
> @@ -135,9 +137,18 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
>  				ret = -ENOMEM;
>  				goto fail;
>  			} else {
> -				src->intr_handle = *intr_handle;
> -				TAILQ_INIT(&src->callbacks);
> -				TAILQ_INSERT_TAIL(&intr_sources, src, next);
> +				src->intr_handle = rte_intr_instance_alloc();
> +				if (src->intr_handle == NULL) {
> +					RTE_LOG(ERR, EAL, "Can not create intr instance\n");
> +					free(callback);
> +					ret = -ENOMEM;

goto fail?

> +				} else {
> +					rte_intr_instance_copy(src->intr_handle,
> +							       intr_handle);
> +					TAILQ_INIT(&src->callbacks);
> +					TAILQ_INSERT_TAIL(&intr_sources, src,
> +							  next);
> +				}
>  			}
>  		}
>  
[...]
> @@ -213,7 +226,7 @@ rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle,
>  	struct rte_intr_callback *cb, *next;
>  
>  	/* do parameter checking first */
> -	if (intr_handle == NULL || intr_handle->fd < 0) {
> +	if (intr_handle == NULL || rte_intr_fd_get(intr_handle) < 0) {

The handle is checked for NULL inside the accessor, here and in other places:
grep -R 'intr_handle == NULL ||' lib/eal

>  		RTE_LOG(ERR, EAL,
>  		"Unregistering with invalid input parameter\n");
>  		return -EINVAL;

> diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
[...]