LinuxLists.cc - drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send

2021-07-07 07:37:54

Subject: drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

tree: ernel.org/pub/scm/linux/kernel/git/torvalds/linux.git">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
4a6616cc4e1c58a58
f26b1512ffc81deed net:CXGB4: fix leak if sk_buff is not used
(attached as .config)
9.3.0-22) 9.3.0
kindly add following tag as appropriate
robot <[email protected]>
<[email protected]>
gb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'
ernet/chelsio/cxgb4/sge.c
2019-11-07 2535 int cxgb4_ethofld_send_flowc(struct net_device *dev, u32 eotid, u32 tc)
2019-11-07 2536 {
2019-11-07 2537 struct port_info *pi = netdev2pinfo(dev);
2019-11-07 2538 struct adapter *adap = netdev2adap(dev);
2019-11-07 2539 enum sge_eosw_state next_state;
2019-11-07 2540 struct sge_eosw_txq *eosw_txq;
2019-11-07 2541 u32 len, len16, nparams = 6;
2019-11-07 2542 struct fw_flowc_wr *flowc;
2019-11-07 2543 struct eotid_entry *entry;
2019-11-07 2544 struct sge_ofld_rxq *rxq;
2019-11-07 2545 struct sk_buff *skb;
2019-11-07 2546 int ret = 0;
2019-11-07 2547
A. R. Silva 2020-06-19 2548 len = struct_size(flowc, mnemval, nparams);
2019-11-07 2549 len16 = DIV_ROUND_UP(len, 16);
2019-11-07 2550
2019-11-07 2551 entry = cxgb4_lookup_eotid(&adap->tids, eotid);
2019-11-07 2552 if (!entry)
2019-11-07 2553 return -ENOMEM;
2019-11-07 2554
2019-11-07 2555 eosw_txq = (struct sge_eosw_txq *)entry->data;
2019-11-07 2556 if (!eosw_txq)
2019-11-07 2557 return -ENOMEM;
2019-11-07 2558
2019-11-07 2559 skb = alloc_skb(len, GFP_KERNEL);
2019-11-07 2560 if (!skb)
2019-11-07 2561 return -ENOMEM;
2019-11-07 2562
2019-11-07 2563 spin_lock_bh(&eosw_txq->lock);
2019-11-07 2564 if (tc != FW_SCHED_CLS_NONE) {
2019-11-07 2565 if (eosw_txq->state != CXGB4_EO_STATE_CLOSED)
2021-05-05 2566 goto out_free_skb;
^^^^^^^^^^^^^^^^^
2019-11-07 2567
2019-11-07 2568 next_state = CXGB4_EO_STATE_FLOWC_OPEN_SEND;
2019-11-07 2569 } else {
2019-11-07 2570 if (eosw_txq->state != CXGB4_EO_STATE_ACTIVE)
2021-05-05 @2571 goto out_free_skb;
2019-11-07 2572
2019-11-07 2573 next_state = CXGB4_EO_STATE_FLOWC_CLOSE_SEND;
2019-11-07 2574 }
2019-11-07 2575
2019-11-07 2576 flowc = __skb_put(skb, len);
2019-11-07 2577 memset(flowc, 0, len);
2019-11-07 2578
2019-11-07 2579 rxq = &adap->sge.eohw_rxq[eosw_txq->hwqid];
2019-11-07 2580 flowc->flowid_len16 = cpu_to_be32(FW_WR_LEN16_V(len16) |
2019-11-07 2581 FW_WR_FLOWID_V(eosw_txq->hwtid));
2019-11-07 2582 flowc->op_to_nparams = cpu_to_be32(FW_WR_OP_V(FW_FLOWC_WR) |
2019-11-07 2583 FW_FLOWC_WR_NPARAMS_V(nparams) |
2019-11-07 2584 FW_WR_COMPL_V(1));
2019-11-07 2585 flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
2019-11-07 2586 flowc->mnemval[0].val = cpu_to_be32(FW_PFVF_CMD_PFN_V(adap->pf));
2019-11-07 2587 flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
2019-11-07 2588 flowc->mnemval[1].val = cpu_to_be32(pi->tx_chan);
2019-11-07 2589 flowc->mnemval[2].mnemonic = FW_FLOWC_MNEM_PORT;
2019-11-07 2590 flowc->mnemval[2].val = cpu_to_be32(pi->tx_chan);
2019-11-07 2591 flowc->mnemval[3].mnemonic = FW_FLOWC_MNEM_IQID;
2019-11-07 2592 flowc->mnemval[3].val = cpu_to_be32(rxq->rspq.abs_id);
2019-11-07 2593 flowc->mnemval[4].mnemonic = FW_FLOWC_MNEM_SCHEDCLASS;
2019-11-07 2594 flowc->mnemval[4].val = cpu_to_be32(tc);
2019-11-07 2595 flowc->mnemval[5].mnemonic = FW_FLOWC_MNEM_EOSTATE;
2019-11-07 2596 flowc->mnemval[5].val = cpu_to_be32(tc == FW_SCHED_CLS_NONE ?
2019-11-07 2597 FW_FLOWC_MNEM_EOSTATE_CLOSING :
2019-11-07 2598 FW_FLOWC_MNEM_EOSTATE_ESTABLISHED);
2019-11-07 2599
2020-04-30 2600 /* Free up any pending skbs to ensure there's room for
2020-04-30 2601 * termination FLOWC.
2020-04-30 2602 */
2020-04-30 2603 if (tc == FW_SCHED_CLS_NONE)
2020-04-30 2604 eosw_txq_flush_pending_skbs(eosw_txq);
2019-11-07 2605
2019-11-07 2606 ret = eosw_txq_enqueue(eosw_txq, skb);
2021-05-05 2607 if (ret)
2021-05-05 2608 goto out_free_skb;
2019-11-07 2609
2019-11-07 2610 eosw_txq->state = next_state;
2019-11-07 2611 eosw_txq->flowc_idx = eosw_txq->pidx;
2019-11-07 2612 eosw_txq_advance(eosw_txq, 1);
2019-11-07 2613 ethofld_xmit(dev, eosw_txq);
2019-11-07 2614
2021-05-05 2615 spin_unlock_bh(&eosw_txq->lock);
2021-05-05 2616 return 0;
2021-05-05 2617
2021-05-05 2618 out_free_skb:
2021-05-05 2619 dev_consume_skb_any(skb);
2019-11-07 2620 spin_unlock_bh(&eosw_txq->lock);
2019-11-07 2621 return ret;
2019-11-07 2622 }
Intel Corporation
hyperkitty/list/kbuild-all@lists.01.org">https://lists.01.org/hyperkitty/list/[email protected]

2021-07-07 08:34:57

by Íñigo Huguet

[permalink] [raw]

Subject: Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

On Wed, Jul 7, 2021 at 9:37 AM Dan Carpenter <[email protected]> wrote:
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2564 if (tc != FW_SCHED_CLS_NONE) {
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2565 if (eosw_txq->state != CXGB4_EO_STATE_CLOSED)
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2566 goto out_free_skb;
> ^^^^^^^^^^^^^^^^^
>
> Are these error paths?
>
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2567
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2568 next_state = CXGB4_EO_STATE_FLOWC_OPEN_SEND;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2569 } else {
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2570 if (eosw_txq->state != CXGB4_EO_STATE_ACTIVE)
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 @2571 goto out_free_skb;
>
> Here too
>
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2572
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2573 next_state = CXGB4_EO_STATE_FLOWC_CLOSE_SEND;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2574 }

I'm not really sure, I just added the skb release in the exit path to
fix a memory leak.

I think it might not be an error path in this case, maybe just no
actions must be done in this specific cases. CCing Raju Rangoju from
Chelsio to see if he can confirm.

--
Íñigo Huguet

2022-03-10 11:36:08

by Íñigo Huguet

[permalink] [raw]

Subject: Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

On Thu, Mar 10, 2022 at 9:00 AM Íñigo Huguet <[email protected]> wrote:
>
> On Wed, Jul 7, 2021 at 9:37 AM Dan Carpenter <[email protected]> wrote:
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 79160a603bdb51916226caf4a6616cc4e1c58a58
> > commit: 52bfcdd87e83d9e69d22da5f26b1512ffc81deed net:CXGB4: fix leak if sk_buff is not used
> > config: x86_64-randconfig-m001-20210706 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> >
> > New smatch warnings:
> > drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'
>
> This was already reported here:
> https://lore.kernel.org/all/[email protected]/
>
> CCing again Chelsio maintainer to see if they can tell whether an
> error code is needed or not. My understanding is that it's not needed
> in this case, but not 100% sure.

I get "The message you sent to [email protected] couldn't be delivered
due to: Recipient email address is possibly incorrect.", but that's
the email listed in MAINTAINERS.

>
> >
> > vim +/ret +2571 drivers/net/ethernet/chelsio/cxgb4/sge.c
> >
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2535 int cxgb4_ethofld_send_flowc(struct net_device *dev, u32 eotid, u32 tc)
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2536 {
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2537 struct port_info *pi = netdev2pinfo(dev);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2538 struct adapter *adap = netdev2adap(dev);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2539 enum sge_eosw_state next_state;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2540 struct sge_eosw_txq *eosw_txq;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2541 u32 len, len16, nparams = 6;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2542 struct fw_flowc_wr *flowc;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2543 struct eotid_entry *entry;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2544 struct sge_ofld_rxq *rxq;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2545 struct sk_buff *skb;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2546 int ret = 0;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2547
> > a422d5ff6defb1 Gustavo A. R. Silva 2020-06-19 2548 len = struct_size(flowc, mnemval, nparams);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2549 len16 = DIV_ROUND_UP(len, 16);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2550
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2551 entry = cxgb4_lookup_eotid(&adap->tids, eotid);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2552 if (!entry)
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2553 return -ENOMEM;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2554
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2555 eosw_txq = (struct sge_eosw_txq *)entry->data;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2556 if (!eosw_txq)
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2557 return -ENOMEM;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2558
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2559 skb = alloc_skb(len, GFP_KERNEL);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2560 if (!skb)
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2561 return -ENOMEM;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2562
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2563 spin_lock_bh(&eosw_txq->lock);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2564 if (tc != FW_SCHED_CLS_NONE) {
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2565 if (eosw_txq->state != CXGB4_EO_STATE_CLOSED)
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2566 goto out_free_skb;
> > ^^^^^^^^^^^^^^^^^
> >
> > Are these error paths?
> >
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2567
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2568 next_state = CXGB4_EO_STATE_FLOWC_OPEN_SEND;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2569 } else {
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2570 if (eosw_txq->state != CXGB4_EO_STATE_ACTIVE)
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 @2571 goto out_free_skb;
> >
> > Here too
> >
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2572
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2573 next_state = CXGB4_EO_STATE_FLOWC_CLOSE_SEND;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2574 }
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2575
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2576 flowc = __skb_put(skb, len);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2577 memset(flowc, 0, len);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2578
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2579 rxq = &adap->sge.eohw_rxq[eosw_txq->hwqid];
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2580 flowc->flowid_len16 = cpu_to_be32(FW_WR_LEN16_V(len16) |
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2581 FW_WR_FLOWID_V(eosw_txq->hwtid));
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2582 flowc->op_to_nparams = cpu_to_be32(FW_WR_OP_V(FW_FLOWC_WR) |
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2583 FW_FLOWC_WR_NPARAMS_V(nparams) |
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2584 FW_WR_COMPL_V(1));
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2585 flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2586 flowc->mnemval[0].val = cpu_to_be32(FW_PFVF_CMD_PFN_V(adap->pf));
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2587 flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2588 flowc->mnemval[1].val = cpu_to_be32(pi->tx_chan);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2589 flowc->mnemval[2].mnemonic = FW_FLOWC_MNEM_PORT;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2590 flowc->mnemval[2].val = cpu_to_be32(pi->tx_chan);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2591 flowc->mnemval[3].mnemonic = FW_FLOWC_MNEM_IQID;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2592 flowc->mnemval[3].val = cpu_to_be32(rxq->rspq.abs_id);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2593 flowc->mnemval[4].mnemonic = FW_FLOWC_MNEM_SCHEDCLASS;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2594 flowc->mnemval[4].val = cpu_to_be32(tc);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2595 flowc->mnemval[5].mnemonic = FW_FLOWC_MNEM_EOSTATE;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2596 flowc->mnemval[5].val = cpu_to_be32(tc == FW_SCHED_CLS_NONE ?
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2597 FW_FLOWC_MNEM_EOSTATE_CLOSING :
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2598 FW_FLOWC_MNEM_EOSTATE_ESTABLISHED);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2599
> > 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2600 /* Free up any pending skbs to ensure there's room for
> > 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2601 * termination FLOWC.
> > 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2602 */
> > 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2603 if (tc == FW_SCHED_CLS_NONE)
> > 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2604 eosw_txq_flush_pending_skbs(eosw_txq);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2605
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2606 ret = eosw_txq_enqueue(eosw_txq, skb);
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2607 if (ret)
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2608 goto out_free_skb;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2609
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2610 eosw_txq->state = next_state;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2611 eosw_txq->flowc_idx = eosw_txq->pidx;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2612 eosw_txq_advance(eosw_txq, 1);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2613 ethofld_xmit(dev, eosw_txq);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2614
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2615 spin_unlock_bh(&eosw_txq->lock);
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2616 return 0;
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2617
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2618 out_free_skb:
> > 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2619 dev_consume_skb_any(skb);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2620 spin_unlock_bh(&eosw_txq->lock);
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2621 return ret;
> > 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2622 }
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/[email protected]
> >
>
>
> --
> Íñigo Huguet

--
Íñigo Huguet

2022-03-10 14:20:44

by Dan Carpenter

[permalink] [raw]

Subject: Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

On Thu, Mar 10, 2022 at 09:00:58AM +0100, ??igo Huguet wrote:
> On Wed, Jul 7, 2021 at 9:37 AM Dan Carpenter <[email protected]> wrote:
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 79160a603bdb51916226caf4a6616cc4e1c58a58
> > commit: 52bfcdd87e83d9e69d22da5f26b1512ffc81deed net:CXGB4: fix leak if sk_buff is not used
> > config: x86_64-randconfig-m001-20210706 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> >
> > New smatch warnings:
> > drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'
>
> This was already reported here:
> https://lore.kernel.org/all/[email protected]/

To be honest, I saw that it was old, and I did wonder why the kbuild bot
was sending warnings from ancient code.

But at the same time, the code looked suspicious enough that I just
resent instead of investigating. :P

With kbuild warnings, I always solemnly promise everyone that "These are
a one time email. Feel free to ignore false positives." Unfortunately
those promises are just lies to make people happy.

regards,
dan carpenter

2022-03-11 22:25:57

by Íñigo Huguet

[permalink] [raw]

Subject: Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

On Wed, Jul 7, 2021 at 9:37 AM Dan Carpenter <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 79160a603bdb51916226caf4a6616cc4e1c58a58
> commit: 52bfcdd87e83d9e69d22da5f26b1512ffc81deed net:CXGB4: fix leak if sk_buff is not used
> config: x86_64-randconfig-m001-20210706 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> New smatch warnings:
> drivers/net/ethernet/chelsio/cxgb4/sge.c:2571 cxgb4_ethofld_send_flowc() warn: missing error code 'ret'

This was already reported here:
https://lore.kernel.org/all/[email protected]/

CCing again Chelsio maintainer to see if they can tell whether an
error code is needed or not. My understanding is that it's not needed
in this case, but not 100% sure.

>
> vim +/ret +2571 drivers/net/ethernet/chelsio/cxgb4/sge.c
>
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2535 int cxgb4_ethofld_send_flowc(struct net_device *dev, u32 eotid, u32 tc)
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2536 {
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2537 struct port_info *pi = netdev2pinfo(dev);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2538 struct adapter *adap = netdev2adap(dev);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2539 enum sge_eosw_state next_state;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2540 struct sge_eosw_txq *eosw_txq;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2541 u32 len, len16, nparams = 6;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2542 struct fw_flowc_wr *flowc;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2543 struct eotid_entry *entry;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2544 struct sge_ofld_rxq *rxq;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2545 struct sk_buff *skb;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2546 int ret = 0;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2547
> a422d5ff6defb1 Gustavo A. R. Silva 2020-06-19 2548 len = struct_size(flowc, mnemval, nparams);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2549 len16 = DIV_ROUND_UP(len, 16);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2550
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2551 entry = cxgb4_lookup_eotid(&adap->tids, eotid);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2552 if (!entry)
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2553 return -ENOMEM;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2554
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2555 eosw_txq = (struct sge_eosw_txq *)entry->data;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2556 if (!eosw_txq)
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2557 return -ENOMEM;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2558
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2559 skb = alloc_skb(len, GFP_KERNEL);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2560 if (!skb)
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2561 return -ENOMEM;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2562
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2563 spin_lock_bh(&eosw_txq->lock);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2564 if (tc != FW_SCHED_CLS_NONE) {
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2565 if (eosw_txq->state != CXGB4_EO_STATE_CLOSED)
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2566 goto out_free_skb;
> ^^^^^^^^^^^^^^^^^
>
> Are these error paths?
>
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2567
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2568 next_state = CXGB4_EO_STATE_FLOWC_OPEN_SEND;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2569 } else {
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2570 if (eosw_txq->state != CXGB4_EO_STATE_ACTIVE)
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 @2571 goto out_free_skb;
>
> Here too
>
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2572
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2573 next_state = CXGB4_EO_STATE_FLOWC_CLOSE_SEND;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2574 }
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2575
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2576 flowc = __skb_put(skb, len);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2577 memset(flowc, 0, len);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2578
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2579 rxq = &adap->sge.eohw_rxq[eosw_txq->hwqid];
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2580 flowc->flowid_len16 = cpu_to_be32(FW_WR_LEN16_V(len16) |
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2581 FW_WR_FLOWID_V(eosw_txq->hwtid));
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2582 flowc->op_to_nparams = cpu_to_be32(FW_WR_OP_V(FW_FLOWC_WR) |
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2583 FW_FLOWC_WR_NPARAMS_V(nparams) |
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2584 FW_WR_COMPL_V(1));
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2585 flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2586 flowc->mnemval[0].val = cpu_to_be32(FW_PFVF_CMD_PFN_V(adap->pf));
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2587 flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2588 flowc->mnemval[1].val = cpu_to_be32(pi->tx_chan);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2589 flowc->mnemval[2].mnemonic = FW_FLOWC_MNEM_PORT;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2590 flowc->mnemval[2].val = cpu_to_be32(pi->tx_chan);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2591 flowc->mnemval[3].mnemonic = FW_FLOWC_MNEM_IQID;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2592 flowc->mnemval[3].val = cpu_to_be32(rxq->rspq.abs_id);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2593 flowc->mnemval[4].mnemonic = FW_FLOWC_MNEM_SCHEDCLASS;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2594 flowc->mnemval[4].val = cpu_to_be32(tc);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2595 flowc->mnemval[5].mnemonic = FW_FLOWC_MNEM_EOSTATE;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2596 flowc->mnemval[5].val = cpu_to_be32(tc == FW_SCHED_CLS_NONE ?
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2597 FW_FLOWC_MNEM_EOSTATE_CLOSING :
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2598 FW_FLOWC_MNEM_EOSTATE_ESTABLISHED);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2599
> 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2600 /* Free up any pending skbs to ensure there's room for
> 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2601 * termination FLOWC.
> 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2602 */
> 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2603 if (tc == FW_SCHED_CLS_NONE)
> 69422a7e5d578a Rahul Lakkireddy 2020-04-30 2604 eosw_txq_flush_pending_skbs(eosw_txq);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2605
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2606 ret = eosw_txq_enqueue(eosw_txq, skb);
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2607 if (ret)
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2608 goto out_free_skb;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2609
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2610 eosw_txq->state = next_state;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2611 eosw_txq->flowc_idx = eosw_txq->pidx;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2612 eosw_txq_advance(eosw_txq, 1);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2613 ethofld_xmit(dev, eosw_txq);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2614
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2615 spin_unlock_bh(&eosw_txq->lock);
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2616 return 0;
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2617
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2618 out_free_skb:
> 52bfcdd87e83d9 Íñigo Huguet 2021-05-05 2619 dev_consume_skb_any(skb);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2620 spin_unlock_bh(&eosw_txq->lock);
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2621 return ret;
> 0e395b3cb1fb82 Rahul Lakkireddy 2019-11-07 2622 }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>

--
Íñigo Huguet