Re: [PATCH v6 2/2] drivers/net: return number of types in get supported types

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 1/25/2024 4:07 PM, Sivaramakrishnan Venkat wrote:
> Missing "RTE_PTYPE_UNKNOWN" ptype causes buffer overflow.
> Enhance code such that the dev_supported_ptypes_get()
> function pointer now returns  the number of elements to
> eliminate the need for "RTE_PTYPE_UNKNOWN" as the last item.
> 
> Signed-off-by: Sivaramakrishnan Venkat <venkatx.sivaramakrishnan@intel.com>
> 

<...>

> @@ -93,7 +93,7 @@ static void axgbe_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
>  	struct rte_eth_rxq_info *qinfo);
>  static void axgbe_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
>  	struct rte_eth_txq_info *qinfo);
> -const uint32_t *axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
> +const uint32_t *axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev, size_t *no_of_elements);
>

I asked to join lines, but it was for the instance overall line length
less than 80 chars, please break the parameter to next line if it is long.
And when breaking please follow the syntax in that driver, some align
the parameter to previous paramter, some uses double/triple tabs.

<...>

> diff --git a/drivers/net/ngbe/ngbe_ptypes.c b/drivers/net/ngbe/ngbe_ptypes.c
> index 33f20e9247..57817e9af1 100644
> --- a/drivers/net/ngbe/ngbe_ptypes.c
> +++ b/drivers/net/ngbe/ngbe_ptypes.c
> @@ -92,7 +92,7 @@ static u32 ngbe_ptype_lookup[NGBE_PTID_MAX] __rte_cache_aligned = {
>  	TPTE(0xCD, ETHER, IPV6, NONE, IP, NONE, IPV6, SCTP),
>  };
>  
> -u32 *ngbe_get_supported_ptypes(void)
> +u32 *ngbe_get_supported_ptypes(size_t *no_of_elements)
>  {
>  	static u32 ptypes[] = {
>  		/* For non-vec functions,
> @@ -110,10 +110,10 @@ u32 *ngbe_get_supported_ptypes(void)
>  		RTE_PTYPE_INNER_L3_IPV6,
>  		RTE_PTYPE_INNER_L3_IPV6_EXT,
>  		RTE_PTYPE_INNER_L4_TCP,
> -		RTE_PTYPE_INNER_L4_UDP,
> -		RTE_PTYPE_UNKNOWN
> +		RTE_PTYPE_INNER_L4_UDP
>

Please keep the terminating ','

<...>

> diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
> index 5a0c3dc4a6..22de4ad13a 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -379,7 +379,7 @@ nicvf_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  }
>  
>  static const uint32_t *
> -nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev)
> +nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev, size_t *no_of_elements)
>  {
>  	size_t copied;
>  	static uint32_t ptypes[32];
> @@ -392,16 +392,13 @@ nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev)
>  		RTE_PTYPE_L4_TCP,
>  		RTE_PTYPE_L4_UDP,
>  		RTE_PTYPE_L4_FRAG,
> -		RTE_PTYPE_UNKNOWN
>  	};
>  	static const uint32_t ptypes_tunnel[] = {
>  		RTE_PTYPE_TUNNEL_GRE,
>  		RTE_PTYPE_TUNNEL_GENEVE,
>  		RTE_PTYPE_TUNNEL_VXLAN,
>  		RTE_PTYPE_TUNNEL_NVGRE,
> -		RTE_PTYPE_UNKNOWN
>  	};
> -	static const uint32_t ptypes_end = RTE_PTYPE_UNKNOWN;
>  
>  	copied = sizeof(ptypes_common);
>  	memcpy(ptypes, ptypes_common, copied);
> @@ -411,9 +408,9 @@ nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev)
>  		copied += sizeof(ptypes_tunnel);
>  	}
>  
> -	memcpy((char *)ptypes + copied, &ptypes_end, sizeof(ptypes_end));
>  
>  	/* All Ptypes are supported in all Rx functions. */
> +	*no_of_elements = RTE_DIM(ptypes);
>

I think this is wrong, won't above always result '32', you should use
'copied' instead.