> -----Original Message----- > From: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > Sent: Tuesday, August 8, 2023 2:11 AM > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > Cc: linux-hyperv@xxxxxxxxxxxxxxx > Subject: [bug report] net: mana: Add page pool for RX buffers > > [You don't often get email from dan.carpenter@xxxxxxxxxx. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > Hello Haiyang Zhang, > > The patch b1d13f7a3b53: "net: mana: Add page pool for RX buffers" > from Aug 4, 2023 (linux-next), leads to the following Smatch static > checker warning: > > drivers/net/ethernet/microsoft/mana/mana_en.c:1651 > mana_process_rx_cqe() > error: uninitialized symbol 'old_fp'. > > drivers/net/ethernet/microsoft/mana/mana_en.c > 1592 static void mana_process_rx_cqe(struct mana_rxq *rxq, struct > mana_cq *cq, > 1593 struct gdma_comp *cqe) > 1594 { > 1595 struct mana_rxcomp_oob *oob = (struct mana_rxcomp_oob > *)cqe->cqe_data; > 1596 struct gdma_context *gc = rxq->gdma_rq->gdma_dev- > >gdma_context; > 1597 struct net_device *ndev = rxq->ndev; > 1598 struct mana_recv_buf_oob *rxbuf_oob; > 1599 struct mana_port_context *apc; > 1600 struct device *dev = gc->dev; > 1601 void *old_buf = NULL; > 1602 u32 curr, pktlen; > 1603 bool old_fp; > 1604 > 1605 apc = netdev_priv(ndev); > 1606 > 1607 switch (oob->cqe_hdr.cqe_type) { > 1608 case CQE_RX_OKAY: > 1609 break; > 1610 > 1611 case CQE_RX_TRUNCATED: > 1612 ++ndev->stats.rx_dropped; > 1613 rxbuf_oob = &rxq->rx_oobs[rxq->buf_index]; > 1614 netdev_warn_once(ndev, "Dropped a truncated packet\n"); > 1615 goto drop; > 1616 > 1617 case CQE_RX_COALESCED_4: > 1618 netdev_err(ndev, "RX coalescing is unsupported\n"); > 1619 apc->eth_stats.rx_coalesced_err++; > 1620 return; > 1621 > 1622 case CQE_RX_OBJECT_FENCE: > 1623 complete(&rxq->fence_event); > 1624 return; > 1625 > 1626 default: > 1627 netdev_err(ndev, "Unknown RX CQE type = %d\n", > 1628 oob->cqe_hdr.cqe_type); > 1629 apc->eth_stats.rx_cqe_unknown_type++; > 1630 return; > 1631 } > 1632 > 1633 pktlen = oob->ppi[0].pkt_len; > 1634 > 1635 if (pktlen == 0) { > 1636 /* data packets should never have packetlength of zero */ > 1637 netdev_err(ndev, "RX pkt len=0, rq=%u, cq=%u, > rxobj=0x%llx\n", > 1638 rxq->gdma_id, cq->gdma_id, rxq->rxobj); > 1639 return; > 1640 } > 1641 > 1642 curr = rxq->buf_index; > 1643 rxbuf_oob = &rxq->rx_oobs[curr]; > 1644 WARN_ON_ONCE(rxbuf_oob->wqe_inf.wqe_size_in_bu != 1); > 1645 > 1646 mana_refill_rx_oob(dev, rxq, rxbuf_oob, &old_buf, &old_fp); > > If mana_get_rxfrag() fails then mana_refill_rx_oob() doesn't set *old_fp. > > > 1647 > 1648 /* Unsuccessful refill will have old_buf == NULL. > 1649 * In this case, mana_rx_skb() will drop the packet. > 1650 */ > --> 1651 mana_rx_skb(old_buf, old_fp, oob, rxq); > 1652 > 1653 drop: > 1654 mana_move_wq_tail(rxq->gdma_rq, rxbuf_oob- > >wqe_inf.wqe_size_in_bu); > 1655 > 1656 mana_post_pkt_rxq(rxq); > 1657 } > > regards, > dan carpenter Yes -- If mana_get_rxfrag() fails then mana_refill_rx_oob() doesn't set *old_fp. But in this case, the old_buf remains NULL, and the following code will drop the packet without accessing the old_fp/from_pool. Do I have to initialize it, or just add a code comment explaining like above? static void mana_rx_skb(void *buf_va, bool from_pool, struct mana_rxcomp_oob *cqe, struct mana_rxq *rxq) { struct mana_stats_rx *rx_stats = &rxq->stats; struct net_device *ndev = rxq->ndev; uint pkt_len = cqe->ppi[0].pkt_len; u16 rxq_idx = rxq->rxq_idx; struct napi_struct *napi; struct xdp_buff xdp = {}; struct sk_buff *skb; u32 hash_value; u32 act; rxq->rx_cq.work_done++; napi = &rxq->rx_cq.napi; if (!buf_va) { ++ndev->stats.rx_dropped; return; } Thanks, - Haiyang
