From e624c70e1131e145bd0510b8a700b5e2d112e377 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:48 +0300 Subject: [PATCH 1/6] bnxt_en: Check devlink allocation and registration status devlink is a software interface that doesn't depend on any hardware capabilities. The failure in SW means memory issues, wrong parameters, programmer error e.t.c. Like any other such interface in the kernel, the returned status of devlink APIs should be checked and propagated further and not ignored. Fixes: 4ab0c6a8ffd7 ("bnxt_en: add support to enable VF-representors") Signed-off-by: Leon Romanovsky Reviewed-by: Edwin Peer Signed-off-by: David S. Miller --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 ++++- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 13 ++++++------- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 13 ------------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 62f84cc91e4d..0fba01db336c 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -13370,7 +13370,9 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) } bnxt_inv_fw_health_reg(bp); - bnxt_dl_register(bp); + rc = bnxt_dl_register(bp); + if (rc) + goto init_err_dl; rc = register_netdev(dev); if (rc) @@ -13390,6 +13392,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) init_err_cleanup: bnxt_dl_unregister(bp); +init_err_dl: bnxt_shutdown_tc(bp); bnxt_clear_int_mode(bp); diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c index bf7d3c17049b..dc0851f709f5 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c @@ -134,7 +134,7 @@ void bnxt_dl_fw_reporters_create(struct bnxt *bp) { struct bnxt_fw_health *health = bp->fw_health; - if (!bp->dl || !health) + if (!health) return; if (!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET) || health->fw_reset_reporter) @@ -188,7 +188,7 @@ void bnxt_dl_fw_reporters_destroy(struct bnxt *bp, bool all) { struct bnxt_fw_health *health = bp->fw_health; - if (!bp->dl || !health) + if (!health) return; if ((all || !(bp->fw_cap & BNXT_FW_CAP_HOT_RESET)) && @@ -781,6 +781,7 @@ int bnxt_dl_register(struct bnxt *bp) { const struct devlink_ops *devlink_ops; struct devlink_port_attrs attrs = {}; + struct bnxt_dl *bp_dl; struct devlink *dl; int rc; @@ -795,7 +796,9 @@ int bnxt_dl_register(struct bnxt *bp) return -ENOMEM; } - bnxt_link_bp_to_dl(bp, dl); + bp->dl = dl; + bp_dl = devlink_priv(dl); + bp_dl->bp = bp; /* Add switchdev eswitch mode setting, if SRIOV supported */ if (pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV) && @@ -826,7 +829,6 @@ int bnxt_dl_register(struct bnxt *bp) err_dl_port_unreg: devlink_port_unregister(&bp->dl_port); err_dl_free: - bnxt_link_bp_to_dl(bp, NULL); devlink_free(dl); return rc; } @@ -835,9 +837,6 @@ void bnxt_dl_unregister(struct bnxt *bp) { struct devlink *dl = bp->dl; - if (!dl) - return; - if (BNXT_PF(bp)) { bnxt_dl_params_unregister(bp); devlink_port_unregister(&bp->dl_port); diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h index d889f240da2b..406dc655a5fc 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h @@ -20,19 +20,6 @@ static inline struct bnxt *bnxt_get_bp_from_dl(struct devlink *dl) return ((struct bnxt_dl *)devlink_priv(dl))->bp; } -/* To clear devlink pointer from bp, pass NULL dl */ -static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl) -{ - bp->dl = dl; - - /* add a back pointer in dl to bp */ - if (dl) { - struct bnxt_dl *bp_dl = devlink_priv(dl); - - bp_dl->bp = bp; - } -} - #define NVM_OFF_MSIX_VEC_PER_PF_MAX 108 #define NVM_OFF_MSIX_VEC_PER_PF_MIN 114 #define NVM_OFF_IGNORE_ARI 164 From 61415c3db3d98c74bfe8f9e8688b6e40b4c3e1d4 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:49 +0300 Subject: [PATCH 2/6] bnxt_en: Properly remove port parameter support This driver doesn't have any port parameters and registers devlink port parameters with empty table. Remove the useless calls to devlink_port_params_register and _unregister. Fixes: da203dfa89ce ("Revert "devlink: Add a generic wake_on_lan port parameter"") Signed-off-by: Leon Romanovsky Reviewed-by: Edwin Peer Signed-off-by: David S. Miller --- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c index dc0851f709f5..ed95e28d60ef 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c @@ -736,9 +736,6 @@ static const struct devlink_param bnxt_dl_params[] = { NULL), }; -static const struct devlink_param bnxt_dl_port_params[] = { -}; - static int bnxt_dl_params_register(struct bnxt *bp) { int rc; @@ -753,14 +750,6 @@ static int bnxt_dl_params_register(struct bnxt *bp) rc); return rc; } - rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params, - ARRAY_SIZE(bnxt_dl_port_params)); - if (rc) { - netdev_err(bp->dev, "devlink_port_params_register failed\n"); - devlink_params_unregister(bp->dl, bnxt_dl_params, - ARRAY_SIZE(bnxt_dl_params)); - return rc; - } devlink_params_publish(bp->dl); return 0; @@ -773,8 +762,6 @@ static void bnxt_dl_params_unregister(struct bnxt *bp) devlink_params_unregister(bp->dl, bnxt_dl_params, ARRAY_SIZE(bnxt_dl_params)); - devlink_port_params_unregister(&bp->dl_port, bnxt_dl_port_params, - ARRAY_SIZE(bnxt_dl_port_params)); } int bnxt_dl_register(struct bnxt *bp) From 42ded61aa75e3432a5363b87a597a43418aa54f2 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:50 +0300 Subject: [PATCH 3/6] devlink: Delete not used port parameters APIs There is no in-kernel users for the devlink port parameters API, so let's remove it. Signed-off-by: Leon Romanovsky Signed-off-by: David S. Miller --- include/net/devlink.h | 6 ------ net/core/devlink.c | 42 ------------------------------------------ 2 files changed, 48 deletions(-) diff --git a/include/net/devlink.h b/include/net/devlink.h index c902e8e5f012..a7852a257bf6 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1653,12 +1653,6 @@ void devlink_param_unregister(struct devlink *devlink, const struct devlink_param *param); void devlink_params_publish(struct devlink *devlink); void devlink_params_unpublish(struct devlink *devlink); -int devlink_port_params_register(struct devlink_port *devlink_port, - const struct devlink_param *params, - size_t params_count); -void devlink_port_params_unregister(struct devlink_port *devlink_port, - const struct devlink_param *params, - size_t params_count); int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id, union devlink_param_value *init_val); int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id, diff --git a/net/core/devlink.c b/net/core/devlink.c index 7d975057c2a9..9c071f4e609f 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -10117,48 +10117,6 @@ void devlink_params_unpublish(struct devlink *devlink) } EXPORT_SYMBOL_GPL(devlink_params_unpublish); -/** - * devlink_port_params_register - register port configuration parameters - * - * @devlink_port: devlink port - * @params: configuration parameters array - * @params_count: number of parameters provided - * - * Register the configuration parameters supported by the port. - */ -int devlink_port_params_register(struct devlink_port *devlink_port, - const struct devlink_param *params, - size_t params_count) -{ - return __devlink_params_register(devlink_port->devlink, - devlink_port->index, - &devlink_port->param_list, params, - params_count, - DEVLINK_CMD_PORT_PARAM_NEW, - DEVLINK_CMD_PORT_PARAM_DEL); -} -EXPORT_SYMBOL_GPL(devlink_port_params_register); - -/** - * devlink_port_params_unregister - unregister port configuration - * parameters - * - * @devlink_port: devlink port - * @params: configuration parameters array - * @params_count: number of parameters provided - */ -void devlink_port_params_unregister(struct devlink_port *devlink_port, - const struct devlink_param *params, - size_t params_count) -{ - return __devlink_params_unregister(devlink_port->devlink, - devlink_port->index, - &devlink_port->param_list, - params, params_count, - DEVLINK_CMD_PORT_PARAM_DEL); -} -EXPORT_SYMBOL_GPL(devlink_port_params_unregister); - static int __devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id, union devlink_param_value *init_val) From 8ba024dfaf6146386d491b4679ba2c54007a7f4f Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:51 +0300 Subject: [PATCH 4/6] devlink: Remove single line function obfuscations There is no need in extra one line functions to call relevant functions only once. Signed-off-by: Leon Romanovsky Signed-off-by: David S. Miller --- net/core/devlink.c | 91 ++++++++++++++++++---------------------------- 1 file changed, 35 insertions(+), 56 deletions(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index 9c071f4e609f..3ea33c689790 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -10117,56 +10117,6 @@ void devlink_params_unpublish(struct devlink *devlink) } EXPORT_SYMBOL_GPL(devlink_params_unpublish); -static int -__devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id, - union devlink_param_value *init_val) -{ - struct devlink_param_item *param_item; - - param_item = devlink_param_find_by_id(param_list, param_id); - if (!param_item) - return -EINVAL; - - if (!param_item->driverinit_value_valid || - !devlink_param_cmode_is_supported(param_item->param, - DEVLINK_PARAM_CMODE_DRIVERINIT)) - return -EOPNOTSUPP; - - if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING) - strcpy(init_val->vstr, param_item->driverinit_value.vstr); - else - *init_val = param_item->driverinit_value; - - return 0; -} - -static int -__devlink_param_driverinit_value_set(struct devlink *devlink, - unsigned int port_index, - struct list_head *param_list, u32 param_id, - union devlink_param_value init_val, - enum devlink_command cmd) -{ - struct devlink_param_item *param_item; - - param_item = devlink_param_find_by_id(param_list, param_id); - if (!param_item) - return -EINVAL; - - if (!devlink_param_cmode_is_supported(param_item->param, - DEVLINK_PARAM_CMODE_DRIVERINIT)) - return -EOPNOTSUPP; - - if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING) - strcpy(param_item->driverinit_value.vstr, init_val.vstr); - else - param_item->driverinit_value = init_val; - param_item->driverinit_value_valid = true; - - devlink_param_notify(devlink, port_index, param_item, cmd); - return 0; -} - /** * devlink_param_driverinit_value_get - get configuration parameter * value for driver initializing @@ -10181,11 +10131,26 @@ __devlink_param_driverinit_value_set(struct devlink *devlink, int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id, union devlink_param_value *init_val) { + struct devlink_param_item *param_item; + if (!devlink_reload_supported(devlink->ops)) return -EOPNOTSUPP; - return __devlink_param_driverinit_value_get(&devlink->param_list, - param_id, init_val); + param_item = devlink_param_find_by_id(&devlink->param_list, param_id); + if (!param_item) + return -EINVAL; + + if (!param_item->driverinit_value_valid || + !devlink_param_cmode_is_supported(param_item->param, + DEVLINK_PARAM_CMODE_DRIVERINIT)) + return -EOPNOTSUPP; + + if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING) + strcpy(init_val->vstr, param_item->driverinit_value.vstr); + else + *init_val = param_item->driverinit_value; + + return 0; } EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_get); @@ -10204,10 +10169,24 @@ EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_get); int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id, union devlink_param_value init_val) { - return __devlink_param_driverinit_value_set(devlink, 0, - &devlink->param_list, - param_id, init_val, - DEVLINK_CMD_PARAM_NEW); + struct devlink_param_item *param_item; + + param_item = devlink_param_find_by_id(&devlink->param_list, param_id); + if (!param_item) + return -EINVAL; + + if (!devlink_param_cmode_is_supported(param_item->param, + DEVLINK_PARAM_CMODE_DRIVERINIT)) + return -EOPNOTSUPP; + + if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING) + strcpy(param_item->driverinit_value.vstr, init_val.vstr); + else + param_item->driverinit_value = init_val; + param_item->driverinit_value_valid = true; + + devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW); + return 0; } EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_set); From 2ff04286a9569675948f39cec2c6ad47c3584633 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:52 +0300 Subject: [PATCH 5/6] ice: Delete always true check of PF pointer PF pointer is always valid when PCI core calls its .shutdown() and .remove() callbacks. There is no need to check it again. Fixes: 837f08fdecbe ("ice: Add basic driver framework for Intel(R) E800 Series") Signed-off-by: Leon Romanovsky Signed-off-by: David S. Miller --- drivers/net/ethernet/intel/ice/ice_main.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 34e64533026a..aacc0b345bbe 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -4593,9 +4593,6 @@ static void ice_remove(struct pci_dev *pdev) struct ice_pf *pf = pci_get_drvdata(pdev); int i; - if (!pf) - return; - for (i = 0; i < ICE_MAX_RESET_WAIT; i++) { if (!ice_is_reset_in_progress(pf->state)) break; From e6a54d6f221301347aaf9d83bb1f23129325c1c5 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Thu, 23 Sep 2021 21:12:53 +0300 Subject: [PATCH 6/6] qed: Don't ignore devlink allocation failures devlink is a software interface that doesn't depend on any hardware capabilities. The failure in SW means memory issues, wrong parameters, programmer error e.t.c. Like any other such interface in the kernel, the returned status of devlink APIs should be checked and propagated further and not ignored. Fixes: 755f982bb1ff ("qed/qede: make devlink survive recovery") Signed-off-by: Leon Romanovsky Signed-off-by: David S. Miller --- drivers/net/ethernet/qlogic/qede/qede_main.c | 12 +++++------- drivers/scsi/qedf/qedf_main.c | 2 ++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c index 9837bdb89cd4..ee4c3bd28a93 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_main.c +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c @@ -1176,19 +1176,17 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level, edev->devlink = qed_ops->common->devlink_register(cdev); if (IS_ERR(edev->devlink)) { DP_NOTICE(edev, "Cannot register devlink\n"); + rc = PTR_ERR(edev->devlink); edev->devlink = NULL; - /* Go on, we can live without devlink */ + goto err3; } } else { struct net_device *ndev = pci_get_drvdata(pdev); + struct qed_devlink *qdl; edev = netdev_priv(ndev); - - if (edev->devlink) { - struct qed_devlink *qdl = devlink_priv(edev->devlink); - - qdl->cdev = cdev; - } + qdl = devlink_priv(edev->devlink); + qdl->cdev = cdev; edev->cdev = cdev; memset(&edev->stats, 0, sizeof(edev->stats)); memcpy(&edev->dev_info, &dev_info, sizeof(dev_info)); diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c index 42d0d941dba5..94ee08fab46a 100644 --- a/drivers/scsi/qedf/qedf_main.c +++ b/drivers/scsi/qedf/qedf_main.c @@ -3416,7 +3416,9 @@ retry_probe: qedf->devlink = qed_ops->common->devlink_register(qedf->cdev); if (IS_ERR(qedf->devlink)) { QEDF_ERR(&qedf->dbg_ctx, "Cannot register devlink\n"); + rc = PTR_ERR(qedf->devlink); qedf->devlink = NULL; + goto err2; } }