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...@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderl...@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.