Bug 188661 - Function bnxt_hwrm_stat_ctx_alloc() returns an improper error code
Summary: Function bnxt_hwrm_stat_ctx_alloc() returns an improper error code
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Network (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_network@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 10:46 UTC by bianpan
Modified: 2017-05-11 09:50 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (1.07 KB, patch)
2017-05-11 09:50 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 10:46:16 UTC
The function bnxt_hwrm_stat_ctx_alloc() defined in drivers/net/ethernet/broadcom/bnxt/bnxt.c should return a non-zero error code when something goes wrong. This can be figured out by checking how its return value is used in function bnxt_init_chip() (at line 4547). However, in function bnxt_hwrm_stat_ctx_alloc(), it returns 0 even when the call to _hwrm_send_message() (at line 4104) fails. Maybe it is better to use "return rc;" instead of "return 0". Codes related to this bug are summarised as follows.

(1) bnxt_hwrm_stat_ctx_alloc @@ drivers/net/ethernet/broadcom/bnxt/bnxt.c
4084 static int bnxt_hwrm_stat_ctx_alloc(struct bnxt *bp)
4085 {
4086     int rc = 0, i;
         ...
4095     req.update_period_ms = cpu_to_le32(bp->stats_coal_ticks / 1000);
4096 
4097     mutex_lock(&bp->hwrm_cmd_lock);
4098     for (i = 0; i < bp->cp_nr_rings; i++) {
4099         struct bnxt_napi *bnapi = bp->bnapi[i];
4100         struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
4101 
4102         req.stats_dma_addr = cpu_to_le64(cpr->hw_stats_map);
4103 
4104         rc = _hwrm_send_message(bp, &req, sizeof(req),
4105                     HWRM_CMD_TIMEOUT);
4106         if (rc)
4107             break;
4108 
4109         cpr->hw_stats_ctx_id = le32_to_cpu(resp->stat_ctx_id);
4110 
4111         bp->grp_info[i].fw_stats_ctx = cpr->hw_stats_ctx_id;
4112     }
4113     mutex_unlock(&bp->hwrm_cmd_lock);
4114     return 0;            // return rc?
4115 }

(2) bnxt_init_chip @@ drivers/net/ethernet/broadcom/bnxt/bnxt.c
4540 static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init)
4541 {
4542     struct bnxt_vnic_info *vnic = &bp->vnic_info[0];
4543     int rc = 0;
4544     unsigned int rx_nr_rings = bp->rx_nr_rings;
4545 
4546     if (irq_re_init) {
4547         rc = bnxt_hwrm_stat_ctx_alloc(bp);
4548         if (rc) {
4549             netdev_err(bp->dev, "hwrm stat ctx alloc failure rc: %x\n",
4550                    rc);
4551             goto err_out;
4552         }
4553     }
         ...
4642 err_out:
4643     bnxt_hwrm_resource_free(bp, 0, true);
4644 
4645     return rc;
4646 }

Thanks very much!
Comment 1 bianpan 2017-05-11 09:50:05 UTC
Created attachment 256389 [details]
The patch fixes the bug

The patch has been merged into the latest version of the Linux kernel. So I will close it.

Note You need to log in before you can comment on or make changes to this bug.