[PATCH v2 01/24] net/nfp: add the stats process logic in ctrl VNIC service

[Ferruh Yigit]

On 10/10/2022 7:08 AM, Chaoyong He wrote:
> Add the flow stats process logic in the ctrl VNIC service.
> The flower firmware pass the flow stats to nfp driver through
> control message, we store them in the flow_priv structure.
> 
> Signed-off-by: Chaoyong He <chaoyong.he at corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at corigine.com>

<...>

> +static void
> +nfp_flower_cmsg_rx_stats(struct nfp_flow_priv *flow_priv,
> +		struct rte_mbuf *mbuf)
> +{
> +	char *msg;
> +	uint16_t i;
> +	uint16_t count;
> +	uint16_t msg_len;
> +	uint32_t ctx_id;
> +	struct nfp_flower_stats_frame *stats;
> +
> +	msg = rte_pktmbuf_mtod(mbuf, char *) + NFP_FLOWER_CMSG_HLEN;
> +	msg_len = mbuf->data_len - NFP_FLOWER_CMSG_HLEN;
> +	count = msg_len / sizeof(struct nfp_flower_stats_frame);
> +
> +	rte_spinlock_lock(&flow_priv->stats_lock);
> +	for (i = 0; i < count; i++) {
> +		stats = (struct nfp_flower_stats_frame *)msg + i;
> +		ctx_id = rte_be_to_cpu_32(stats->stats_con_id);
> +		flow_priv->stats[ctx_id].pkts  += rte_be_to_cpu_32(stats->pkt_count);
> +		flow_priv->stats[ctx_id].bytes += rte_be_to_cpu_64(stats->byte_count);

Like below comment, 'stats' is a pointer and I don't see any code that 
allocates memory that this pointer points, at this stage it has invalid 
value and above code will crash.

I assume that memory allocation is in next patch, in that case this 
patch should come after it.

<...>

>   void
>   nfp_flower_ctrl_vnic_poll(struct nfp_app_fw_flower *app_fw_flower)
>   {
> -	uint16_t i;
>   	uint16_t count;
>   	struct nfp_net_rxq *rxq;
>   	struct nfp_net_hw *ctrl_hw;
> @@ -242,9 +308,8 @@
>   		count = nfp_flower_ctrl_vnic_recv(rxq, pkts_burst, MAX_PKT_BURST);
>   		if (count != 0) {
>   			app_fw_flower->ctrl_vnic_rx_count += count;
> -			/* Process cmsgs here, only free for now */
> -			for (i = 0; i < count; i++)
> -				rte_pktmbuf_free(pkts_burst[i]);
> +			/* Process cmsgs here */
> +			nfp_flower_cmsg_rx(app_fw_flower->flow_priv, pkts_burst, count);

At this point, as far as I can see, 'app_fw_flower->flow_priv' is not 
pointing to any allocated memory, so it is an invalid pointer.