Re: [PATCH v4 net-next 04/14] net: enetc: add basic operations to the FDB table

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: enetc: add basic operations to the FDB table

This commit adds FDB table management operations (Add, Delete, Update,
Query/Search) to the enetc driver using NTMP version 2.0. FDB entries
are managed via the command BD ring.

> diff --git a/include/linux/fsl/ntmp.h b/include/linux/fsl/ntmp.h
> index 916dc4fe7de3..a9f3e6cbf422 100644
> --- a/include/linux/fsl/ntmp.h
> +++ b/include/linux/fsl/ntmp.h

[ ... ]

> @@ -116,6 +158,31 @@ static inline int ntmp_rsst_query_entry(struct ntmp_user *user,
>  	return 0;
>  }
>  
> +static inline int ntmp_fdbt_add_entry(struct ntmp_user *user, u32 *entry_id,
> +				      const struct fdbt_keye_data *keye,
> +				      const struct fdbt_cfge_data *data)
> +{
> +	return 0;
> +}

If CONFIG_NXP_NETC_LIB is disabled, will returning 0 here cause callers to
falsely assume the operation succeeded and proceed to use an uninitialized
entry_id pointer? Would it be safer to return an error code such as
-EOPNOTSUPP for stubs that have output parameters?

[ ... ]

> +static inline int ntmp_fdbt_search_port_entry(struct ntmp_user *user, int port,
> +					      u32 *resume_entry_id,
> +					      struct fdbt_entry_data *entry)
> +{
> +	return 0;
> +}

Similar to the above, does returning 0 here leave both resume_entry_id and
entry uninitialized while signaling success? Since resume_entry_id is
typically used as an iterator, could this lead to an infinite loop or other
unpredictable behavior in the caller if they interpret the 0 return as a
successful read of valid hardware configuration data?