Skip to content

Commit bd6781c

Browse files
Kalesh APkuba-moo
authored andcommitted
bnxt_en: Fix wrong return value check in bnxt_close_nic()
The wait_event_interruptible_timeout() function returns 0 if the timeout elapsed, -ERESTARTSYS if it was interrupted by a signal, and the remaining jiffies otherwise if the condition evaluated to true before the timeout elapsed. Driver should have checked for zero return value instead of a positive value. MChan: Print a warning for -ERESTARTSYS. The close operation will proceed anyway when wait_event_interruptible_timeout() returns for any reason. Since we do the close no matter what, we should not return this error code to the caller. Change bnxt_close_nic() to a void function and remove all error handling from some of the callers. Fixes: c0c050c ("bnxt_en: New Broadcom ethernet driver.") Reviewed-by: Andy Gospodarek <[email protected]> Reviewed-by: Vikas Gupta <[email protected]> Reviewed-by: Somnath Kotur <[email protected]> Signed-off-by: Kalesh AP <[email protected]> Signed-off-by: Michael Chan <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent aded5d1 commit bd6781c

File tree

5 files changed

+16
-34
lines changed

5 files changed

+16
-34
lines changed

drivers/net/ethernet/broadcom/bnxt/bnxt.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10732,10 +10732,8 @@ static void __bnxt_close_nic(struct bnxt *bp, bool irq_re_init,
1073210732
bnxt_free_mem(bp, irq_re_init);
1073310733
}
1073410734

10735-
int bnxt_close_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
10735+
void bnxt_close_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
1073610736
{
10737-
int rc = 0;
10738-
1073910737
if (test_bit(BNXT_STATE_IN_FW_RESET, &bp->state)) {
1074010738
/* If we get here, it means firmware reset is in progress
1074110739
* while we are trying to close. We can safely proceed with
@@ -10750,15 +10748,18 @@ int bnxt_close_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
1075010748

1075110749
#ifdef CONFIG_BNXT_SRIOV
1075210750
if (bp->sriov_cfg) {
10751+
int rc;
10752+
1075310753
rc = wait_event_interruptible_timeout(bp->sriov_cfg_wait,
1075410754
!bp->sriov_cfg,
1075510755
BNXT_SRIOV_CFG_WAIT_TMO);
10756-
if (rc)
10757-
netdev_warn(bp->dev, "timeout waiting for SRIOV config operation to complete!\n");
10756+
if (!rc)
10757+
netdev_warn(bp->dev, "timeout waiting for SRIOV config operation to complete, proceeding to close!\n");
10758+
else if (rc < 0)
10759+
netdev_warn(bp->dev, "SRIOV config operation interrupted, proceeding to close!\n");
1075810760
}
1075910761
#endif
1076010762
__bnxt_close_nic(bp, irq_re_init, link_re_init);
10761-
return rc;
1076210763
}
1076310764

1076410765
static int bnxt_close(struct net_device *dev)

drivers/net/ethernet/broadcom/bnxt/bnxt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2375,7 +2375,7 @@ int bnxt_open_nic(struct bnxt *, bool, bool);
23752375
int bnxt_half_open_nic(struct bnxt *bp);
23762376
void bnxt_half_close_nic(struct bnxt *bp);
23772377
void bnxt_reenable_sriov(struct bnxt *bp);
2378-
int bnxt_close_nic(struct bnxt *, bool, bool);
2378+
void bnxt_close_nic(struct bnxt *, bool, bool);
23792379
void bnxt_get_ring_err_stats(struct bnxt *bp,
23802380
struct bnxt_total_ring_err_stats *stats);
23812381
int bnxt_dbg_hwrm_rd_reg(struct bnxt *bp, u32 reg_off, u16 num_words,

drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -449,15 +449,8 @@ static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
449449
return -ENODEV;
450450
}
451451
bnxt_ulp_stop(bp);
452-
if (netif_running(bp->dev)) {
453-
rc = bnxt_close_nic(bp, true, true);
454-
if (rc) {
455-
NL_SET_ERR_MSG_MOD(extack, "Failed to close");
456-
dev_close(bp->dev);
457-
rtnl_unlock();
458-
break;
459-
}
460-
}
452+
if (netif_running(bp->dev))
453+
bnxt_close_nic(bp, true, true);
461454
bnxt_vf_reps_free(bp);
462455
rc = bnxt_hwrm_func_drv_unrgtr(bp);
463456
if (rc) {

drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,8 @@ static int bnxt_set_coalesce(struct net_device *dev,
165165
reset_coalesce:
166166
if (test_bit(BNXT_STATE_OPEN, &bp->state)) {
167167
if (update_stats) {
168-
rc = bnxt_close_nic(bp, true, false);
169-
if (!rc)
170-
rc = bnxt_open_nic(bp, true, false);
168+
bnxt_close_nic(bp, true, false);
169+
rc = bnxt_open_nic(bp, true, false);
171170
} else {
172171
rc = bnxt_hwrm_set_coal(bp);
173172
}
@@ -972,12 +971,7 @@ static int bnxt_set_channels(struct net_device *dev,
972971
* before PF unload
973972
*/
974973
}
975-
rc = bnxt_close_nic(bp, true, false);
976-
if (rc) {
977-
netdev_err(bp->dev, "Set channel failure rc :%x\n",
978-
rc);
979-
return rc;
980-
}
974+
bnxt_close_nic(bp, true, false);
981975
}
982976

983977
if (sh) {
@@ -4042,12 +4036,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
40424036
bnxt_run_fw_tests(bp, test_mask, &test_results);
40434037
} else {
40444038
bnxt_ulp_stop(bp);
4045-
rc = bnxt_close_nic(bp, true, false);
4046-
if (rc) {
4047-
etest->flags |= ETH_TEST_FL_FAILED;
4048-
bnxt_ulp_start(bp, rc);
4049-
return;
4050-
}
4039+
bnxt_close_nic(bp, true, false);
40514040
bnxt_run_fw_tests(bp, test_mask, &test_results);
40524041

40534042
buf[BNXT_MACLPBK_TEST_IDX] = 1;

drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,8 @@ static int bnxt_hwrm_ptp_cfg(struct bnxt *bp)
521521

522522
if (netif_running(bp->dev)) {
523523
if (ptp->rx_filter == HWTSTAMP_FILTER_ALL) {
524-
rc = bnxt_close_nic(bp, false, false);
525-
if (!rc)
526-
rc = bnxt_open_nic(bp, false, false);
524+
bnxt_close_nic(bp, false, false);
525+
rc = bnxt_open_nic(bp, false, false);
527526
} else {
528527
bnxt_ptp_cfg_tstamp_filters(bp);
529528
}

0 commit comments

Comments
 (0)