[dpdk-dev] [PATCH v5 2/6] eal/interrupts: avoid direct access to interrupt handle

[Dmitry Kozlyuk]

2021-10-23 02:19 (UTC+0530), Harman Kalra:
> Making changes to the interrupt framework to use interrupt handle
> APIs to get/set any field.
> 
> Signed-off-by: Harman Kalra <hkalra at marvell.com>
> ---
>  lib/eal/freebsd/eal_interrupts.c | 112 ++++++++----
>  lib/eal/linux/eal_interrupts.c   | 303 +++++++++++++++++++------------
>  2 files changed, 268 insertions(+), 147 deletions(-)
> 
> diff --git a/lib/eal/freebsd/eal_interrupts.c b/lib/eal/freebsd/eal_interrupts.c
> [...]
> +				/* src->interrupt instance memory allocated
> +				 * depends on from where intr_handle memory
> +				 * is allocated.
> +				 */
> +				is_rte_memory =
> +				!!(rte_intr_instance_alloc_flag_get(
> +				intr_handle) & RTE_INTR_INSTANCE_F_SHARED);
> +				if (is_rte_memory == 0)
> +					src->intr_handle =
> +						rte_intr_instance_alloc(
> +						RTE_INTR_INSTANCE_F_UNSHARED);
> +				else if (is_rte_memory == 1)
> +					src->intr_handle =
> +						rte_intr_instance_alloc(
> +						RTE_INTR_INSTANCE_F_SHARED);
> +				else
> +					RTE_LOG(ERR, EAL, "Failed to get mem allocator\n");

Why not just get the flags and use them as-is to allocate a new instance?
If you care to use only these flags even if there are others,
a mask can be used.

> +
> +				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);
> +				}
>  			}
>  		}
>  [...]

> diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
> [...]
> @@ -522,12 +547,35 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
>  			free(callback);
>  			ret = -ENOMEM;
>  		} else {
> -			src->intr_handle = *intr_handle;
> -			TAILQ_INIT(&src->callbacks);
> -			TAILQ_INSERT_TAIL(&(src->callbacks), callback, next);
> -			TAILQ_INSERT_TAIL(&intr_sources, src, next);
> -			wake_thread = 1;
> -			ret = 0;
> +			/* src->interrupt instance memory allocated depends on
> +			 * from where intr_handle memory is allocated.
> +			 */
> +			is_rte_memory =
> +			!!(rte_intr_instance_alloc_flag_get(intr_handle) &
> +					RTE_INTR_INSTANCE_F_SHARED);
> +			if (is_rte_memory == 0)
> +				src->intr_handle = rte_intr_instance_alloc(
> +						RTE_INTR_INSTANCE_F_UNSHARED);
> +			else if (is_rte_memory == 1)
> +				src->intr_handle = rte_intr_instance_alloc(
> +						RTE_INTR_INSTANCE_F_SHARED);
> +			else
> +				RTE_LOG(ERR, EAL, "Failed to get mem allocator\n");

Likewise.

> +
> +			if (src->intr_handle == NULL) {
> +				RTE_LOG(ERR, EAL, "Can not create intr instance\n");
> +				free(callback);
> +				ret = -ENOMEM;
> +			} else {
> +				rte_intr_instance_copy(src->intr_handle,
> +						       intr_handle);
> +				TAILQ_INIT(&src->callbacks);
> +				TAILQ_INSERT_TAIL(&(src->callbacks), callback,
> +						  next);
> +				TAILQ_INSERT_TAIL(&intr_sources, src, next);
> +				wake_thread = 1;
> +				ret = 0;
> +			}
>  		}
>  	}
>  

[...]
> +	if (intr_handle && rte_intr_type_get(intr_handle) ==
> +							RTE_INTR_HANDLE_VDEV)
>  		return 0;

Nit: you have removed `intr_handle` condition everywhere except here.