[1/2] net/bnxt: fix resource cleanup
Checks
Commit Message
Fix resource cleanup in port close.
Once the pointers are freed, set them to NULL.
Make sure access to the pointers is validated before use.
Fixes: bb81e07323bb ("net/bnxt: support LED on/off")
Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
Fixes: 1d0704f4d793 ("net/bnxt: add device configure operation")
Fixes: 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
Fixes: 322bd6e70272 ("net/bnxt: add port representor infrastructure")
Fixes: 0bf5a0b5ebb8 ("net/bnxt: add a failure log")
Cc: stable@dpdk.org
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
drivers/net/bnxt/bnxt_ethdev.c | 39 +++++++++++++++++++++++++---------
drivers/net/bnxt/bnxt_hwrm.c | 3 +++
2 files changed, 32 insertions(+), 10 deletions(-)
Comments
On Mon, Apr 19, 2021 at 12:28 PM Ajit Khaparde
<ajit.khaparde@broadcom.com> wrote:
>
> Fix resource cleanup in port close.
> Once the pointers are freed, set them to NULL.
> Make sure access to the pointers is validated before use.
>
> Fixes: bb81e07323bb ("net/bnxt: support LED on/off")
> Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
> Fixes: 1d0704f4d793 ("net/bnxt: add device configure operation")
> Fixes: 698aa7e95325 ("net/bnxt: add code to determine the Tx COS queue")
> Fixes: 322bd6e70272 ("net/bnxt: add port representor infrastructure")
> Fixes: 0bf5a0b5ebb8 ("net/bnxt: add a failure log")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Patch applied to dpdk-next-net-brcm.
> ---
> drivers/net/bnxt/bnxt_ethdev.c | 39 +++++++++++++++++++++++++---------
> drivers/net/bnxt/bnxt_hwrm.c | 3 +++
> 2 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 20ca57eae5..dc3b04110f 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -219,16 +219,19 @@ uint16_t bnxt_rss_hash_tbl_size(const struct bnxt *bp)
> static void bnxt_free_parent_info(struct bnxt *bp)
> {
> rte_free(bp->parent);
> + bp->parent = NULL;
> }
>
> static void bnxt_free_pf_info(struct bnxt *bp)
> {
> rte_free(bp->pf);
> + bp->pf = NULL;
> }
>
> static void bnxt_free_link_info(struct bnxt *bp)
> {
> rte_free(bp->link_info);
> + bp->link_info = NULL;
> }
>
> static void bnxt_free_leds_info(struct bnxt *bp)
> @@ -249,7 +252,9 @@ static void bnxt_free_flow_stats_info(struct bnxt *bp)
> static void bnxt_free_cos_queues(struct bnxt *bp)
> {
> rte_free(bp->rx_cos_queue);
> + bp->rx_cos_queue = NULL;
> rte_free(bp->tx_cos_queue);
> + bp->tx_cos_queue = NULL;
> }
>
> static void bnxt_free_mem(struct bnxt *bp, bool reconfig)
> @@ -849,9 +854,14 @@ static int bnxt_shutdown_nic(struct bnxt *bp)
>
> uint32_t bnxt_get_speed_capabilities(struct bnxt *bp)
> {
> - uint32_t link_speed = bp->link_info->support_speeds;
> + uint32_t link_speed = 0;
> uint32_t speed_capa = 0;
>
> + if (bp->link_info == NULL)
> + return 0;
> +
> + link_speed = bp->link_info->support_speeds;
> +
> /* If PAM4 is configured, use PAM4 supported speed */
> if (link_speed == 0 && bp->link_info->support_pam4_speeds > 0)
> link_speed = bp->link_info->support_pam4_speeds;
> @@ -1293,12 +1303,13 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
> {
> int rc = 0;
>
> - if (bp->switch_domain_id) {
> - rc = rte_eth_switch_domain_free(bp->switch_domain_id);
> - if (rc)
> - PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
> - bp->switch_domain_id, rc);
> - }
> + if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp)))
> + return;
> +
> + rc = rte_eth_switch_domain_free(bp->switch_domain_id);
> + if (rc)
> + PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
> + bp->switch_domain_id, rc);
> }
>
> static void bnxt_ptp_get_current_time(void *arg)
> @@ -1545,11 +1556,9 @@ bnxt_uninit_locks(struct bnxt *bp)
>
> static void bnxt_drv_uninit(struct bnxt *bp)
> {
> - bnxt_free_switch_domain(bp);
> bnxt_free_leds_info(bp);
> bnxt_free_cos_queues(bp);
> bnxt_free_link_info(bp);
> - bnxt_free_pf_info(bp);
> bnxt_free_parent_info(bp);
> bnxt_uninit_locks(bp);
>
> @@ -1559,6 +1568,7 @@ static void bnxt_drv_uninit(struct bnxt *bp)
> bp->rx_mem_zone = NULL;
>
> bnxt_free_vf_info(bp);
> + bnxt_free_pf_info(bp);
>
> rte_free(bp->grp_info);
> bp->grp_info = NULL;
> @@ -1720,6 +1730,10 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
> return rc;
>
> memset(&new, 0, sizeof(new));
> +
> + if (bp->link_info == NULL)
> + goto out;
> +
> do {
> /* Retrieve link info from hardware */
> rc = bnxt_get_hwrm_link_config(bp, &new);
> @@ -5748,7 +5762,10 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
> bnxt_free_mem(bp, reconfig_dev);
>
> bnxt_hwrm_func_buf_unrgtr(bp);
> - rte_free(bp->pf->vf_req_buf);
> + if (bp->pf != NULL) {
> + rte_free(bp->pf->vf_req_buf);
> + bp->pf->vf_req_buf = NULL;
> + }
>
> rc = bnxt_hwrm_func_driver_unregister(bp, 0);
> bp->flags &= ~BNXT_FLAG_REGISTERED;
> @@ -5761,6 +5778,8 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
> bnxt_uninit_ctx_mem(bp);
>
> bnxt_free_flow_stats_info(bp);
> + if (bp->rep_info != NULL)
> + bnxt_free_switch_domain(bp);
> bnxt_free_rep_info(bp);
> rte_free(bp->ptp_cfg);
> bp->ptp_cfg = NULL;
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index a119fc3a0b..242493982c 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -795,6 +795,9 @@ void bnxt_free_vf_info(struct bnxt *bp)
> if (bp->pf->vf_info == NULL)
> return;
>
> + if (bp->pf == NULL)
> + return;
> +
> for (i = 0; i < bp->pf->max_vfs; i++) {
> rte_free(bp->pf->vf_info[i].vlan_table);
> bp->pf->vf_info[i].vlan_table = NULL;
> --
> 2.21.1 (Apple Git-122.3)
>
@@ -219,16 +219,19 @@ uint16_t bnxt_rss_hash_tbl_size(const struct bnxt *bp)
static void bnxt_free_parent_info(struct bnxt *bp)
{
rte_free(bp->parent);
+ bp->parent = NULL;
}
static void bnxt_free_pf_info(struct bnxt *bp)
{
rte_free(bp->pf);
+ bp->pf = NULL;
}
static void bnxt_free_link_info(struct bnxt *bp)
{
rte_free(bp->link_info);
+ bp->link_info = NULL;
}
static void bnxt_free_leds_info(struct bnxt *bp)
@@ -249,7 +252,9 @@ static void bnxt_free_flow_stats_info(struct bnxt *bp)
static void bnxt_free_cos_queues(struct bnxt *bp)
{
rte_free(bp->rx_cos_queue);
+ bp->rx_cos_queue = NULL;
rte_free(bp->tx_cos_queue);
+ bp->tx_cos_queue = NULL;
}
static void bnxt_free_mem(struct bnxt *bp, bool reconfig)
@@ -849,9 +854,14 @@ static int bnxt_shutdown_nic(struct bnxt *bp)
uint32_t bnxt_get_speed_capabilities(struct bnxt *bp)
{
- uint32_t link_speed = bp->link_info->support_speeds;
+ uint32_t link_speed = 0;
uint32_t speed_capa = 0;
+ if (bp->link_info == NULL)
+ return 0;
+
+ link_speed = bp->link_info->support_speeds;
+
/* If PAM4 is configured, use PAM4 supported speed */
if (link_speed == 0 && bp->link_info->support_pam4_speeds > 0)
link_speed = bp->link_info->support_pam4_speeds;
@@ -1293,12 +1303,13 @@ static void bnxt_free_switch_domain(struct bnxt *bp)
{
int rc = 0;
- if (bp->switch_domain_id) {
- rc = rte_eth_switch_domain_free(bp->switch_domain_id);
- if (rc)
- PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
- bp->switch_domain_id, rc);
- }
+ if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp)))
+ return;
+
+ rc = rte_eth_switch_domain_free(bp->switch_domain_id);
+ if (rc)
+ PMD_DRV_LOG(ERR, "free switch domain:%d fail: %d\n",
+ bp->switch_domain_id, rc);
}
static void bnxt_ptp_get_current_time(void *arg)
@@ -1545,11 +1556,9 @@ bnxt_uninit_locks(struct bnxt *bp)
static void bnxt_drv_uninit(struct bnxt *bp)
{
- bnxt_free_switch_domain(bp);
bnxt_free_leds_info(bp);
bnxt_free_cos_queues(bp);
bnxt_free_link_info(bp);
- bnxt_free_pf_info(bp);
bnxt_free_parent_info(bp);
bnxt_uninit_locks(bp);
@@ -1559,6 +1568,7 @@ static void bnxt_drv_uninit(struct bnxt *bp)
bp->rx_mem_zone = NULL;
bnxt_free_vf_info(bp);
+ bnxt_free_pf_info(bp);
rte_free(bp->grp_info);
bp->grp_info = NULL;
@@ -1720,6 +1730,10 @@ int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
return rc;
memset(&new, 0, sizeof(new));
+
+ if (bp->link_info == NULL)
+ goto out;
+
do {
/* Retrieve link info from hardware */
rc = bnxt_get_hwrm_link_config(bp, &new);
@@ -5748,7 +5762,10 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
bnxt_free_mem(bp, reconfig_dev);
bnxt_hwrm_func_buf_unrgtr(bp);
- rte_free(bp->pf->vf_req_buf);
+ if (bp->pf != NULL) {
+ rte_free(bp->pf->vf_req_buf);
+ bp->pf->vf_req_buf = NULL;
+ }
rc = bnxt_hwrm_func_driver_unregister(bp, 0);
bp->flags &= ~BNXT_FLAG_REGISTERED;
@@ -5761,6 +5778,8 @@ bnxt_uninit_resources(struct bnxt *bp, bool reconfig_dev)
bnxt_uninit_ctx_mem(bp);
bnxt_free_flow_stats_info(bp);
+ if (bp->rep_info != NULL)
+ bnxt_free_switch_domain(bp);
bnxt_free_rep_info(bp);
rte_free(bp->ptp_cfg);
bp->ptp_cfg = NULL;
@@ -795,6 +795,9 @@ void bnxt_free_vf_info(struct bnxt *bp)
if (bp->pf->vf_info == NULL)
return;
+ if (bp->pf == NULL)
+ return;
+
for (i = 0; i < bp->pf->max_vfs; i++) {
rte_free(bp->pf->vf_info[i].vlan_table);
bp->pf->vf_info[i].vlan_table = NULL;