Re: [PATCH i-g-t 4/5] lib/igt_sriov_device: Add helper functions for VF range validation

["Laguna, Lukasz" <lukasz.laguna@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5465 bytes --]

On 12/18/2024 13:00, Marcin Bernatowicz wrote:
> Add __is_valid_range() to check if a VF range is valid. Introduce
> igt_sriov_random_vf_in_range() to get a random VF number within a
> specified range. Update for_random_sriov_vf to use the new helper
> functions for better range handling.
>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@intel.com>
> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
> Cc: Michał Winiarski<michal.winiarski@intel.com>
> Cc: Narasimha C V<narasimha.c.v@intel.com>
> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
> Cc: Satyanarayana K V P<satyanarayana.k.v.p@intel.com>
> Cc: Tomasz Lis<tomasz.lis@intel.com>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
> ---
>   lib/igt_sriov_device.h | 82 ++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 75 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_sriov_device.h b/lib/igt_sriov_device.h
> index 4b63ceb22..de25a7d98 100644
> --- a/lib/igt_sriov_device.h
> +++ b/lib/igt_sriov_device.h
> @@ -34,6 +34,45 @@ int igt_sriov_device_sysfs_open(int pf, unsigned int vf_num);
>   bool igt_sriov_device_reset_exists(int pf, unsigned int vf_num);
>   bool igt_sriov_device_reset(int pf, unsigned int vf_num);
>   
> +/**
> + * __is_valid_range - Helper to check VF range is valid
> + * @start_vf: Starting VF number
> + * @end_vf: Ending VF number
> + * @total_vfs: Total number of VFs
> + *
> + * Return: true if the range is valid, false otherwise.
> + */
> +static inline bool __is_valid_range(unsigned int start_vf, unsigned int end_vf,
> +				    unsigned int total_vfs)
> +{
> +	return !igt_warn_on_f(start_vf > end_vf || end_vf > total_vfs || start_vf == 0,
> +			      "start_vf=%u, end_vf=%u, total_vfs=%u\n",
> +			      start_vf, end_vf, total_vfs);
> +}
> +
> +/**
> + * igt_sriov_random_vf_in_range - Get a random VF number within a specified range
> + * @pf_fd: PF device file descriptor
> + * @start: Starting VF number in the range
> + * @end: Ending VF number in the range
> + *
> + * Returns a random VF number within the specified range [start, end].
> + * If the range is invalid (start > end, end > total VFs,
> + * or start == 0), the function returns 0.
> + *
> + * Return: A random VF number within the range, or 0 if the range is invalid.
> + */
> +static inline unsigned int
> +igt_sriov_random_vf_in_range(int pf_fd, unsigned int start, unsigned int end)
> +{
> +	unsigned int total_vfs = igt_sriov_get_total_vfs(pf_fd);
> +
> +	if (!__is_valid_range(start, end, total_vfs))
> +		return 0;
> +
> +	return start + random() % (end - start + 1);
> +}
> +
>   /**
>    * for_each_sriov_vf - Helper for running code on each VF
>    * @__pf_fd: PF device file descriptor
> @@ -48,17 +87,46 @@ bool igt_sriov_device_reset(int pf, unsigned int vf_num);
>   #define for_each_sriov_num_vfs for_each_sriov_vf
>   
>   /**
> - * for_random_sriov_vf - Helper for running code on random VF
> + * for_random_sriov_vf_in_range - Iterate over a random VF in a specified range
> + * @__pf_fd: PF device file descriptor
> + * @__start: Starting VF number in the range
> + * @__end: Ending VF number in the range
> + * @__vf_num: Variable to store the random VF number
> + *
> + * Iterates over a random VF number within the specified range [__start, __end].
> + * The loop runs only if the range is valid and a random
> + * VF number is successfully selected.
> + */
> +#define for_random_sriov_vf_in_range(__pf_fd, __start, __end, __vf_num) \
> +	for (unsigned int __vf_num = igt_sriov_random_vf_in_range(__pf_fd, __start, __end); \
> +	     __vf_num != 0; __vf_num = 0)
> +
> +/**
> + * for_random_sriov_vf_starting_from - Iterate over a random VF starting from a specified VF
> + * @__pf_fd: PF device file descriptor
> + * @__start: Starting VF number
> + * @__vf_num: Variable to store the random VF number
> + *
> + * This macro iterates over a random VF number starting from the specified
> + * VF number @__start to the total number of VFs associated with the given
> + * PF @__pf_fd.
> + */
> +#define for_random_sriov_vf_starting_from(__pf_fd, __start, __vf_num) \
> +	for_random_sriov_vf_in_range(__pf_fd, __start, igt_sriov_get_total_vfs(__pf_fd), __vf_num)
> +
> +/**
> + * for_random_sriov_vf - Iterate over a random VF for a given PF
>    * @__pf_fd: PF device file descriptor
> - * @__vf_num: stores random VF
> + * @__vf_num: Variable to store the random VF number
>    *
> - * Helper allows to run code using random VF number (stored in @__vf_num)
> - * picked from the range of all VFs associated with given PF @__pf_fd.
> + * Iterates over a random VF number selected from the range
> + * of all VFs associated with the given PF @__pf_fd. The loop runs only
> + * if a random VF number is successfully selected.
>    */
>   #define for_random_sriov_vf(__pf_fd, __vf_num) \
> -	for (unsigned int __vf_num = 1 + random() % igt_sriov_get_total_vfs(__pf_fd), __tmp = 0; \
> -	     __tmp < 1; \
> -	     ++__tmp)
> +	for_random_sriov_vf_in_range(__pf_fd, 1, igt_sriov_get_total_vfs(__pf_fd), __vf_num)
> +
> +/* for_random_sriov_num_vfs - Alias for for_random_sriov_vf */
>   #define for_random_sriov_num_vfs for_random_sriov_vf
>   
>   /**

You've added your signed-off twice.
Apart from that all looks good:
Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com>


[-- Attachment #2: Type: text/html, Size: 6824 bytes --]