[PATCH rdma-rc 0/4] bnxt_re bug fixes

[PATCH rdma-rc 0/4] bnxt_re bug fixes

[Kalesh AP]

This series contains bug fixes related to debugfs
support in the driver. One fix is for preventing
a NULL pointer dereference in reset recovery path.

Please review and apply.

Regards,
Kalesh AP

Gautam R A (2):
  RDMA/bnxt_re: Fix incorrect display of inactivity_cp in debugfs output
  RDMA/bnxt_re: Fix missing error handling for tx_queue

Kalesh AP (2):
  RDMA/bnxt_re: Fix return code of bnxt_re_configure_cc
  RDMA/bnxt_re: Fix null pointer dereference in bnxt_re_suspend

 drivers/infiniband/hw/bnxt_re/debugfs.c | 20 +++++++++++++++-----
 drivers/infiniband/hw/bnxt_re/main.c    |  2 ++
 2 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.43.5

[PATCH rdma-rc 1/4] RDMA/bnxt_re: Fix incorrect display of inactivity_cp in debugfs output

[Kalesh AP]

From: Gautam R A <gautam-r.a@broadcom.com>

The inactivity_cp parameter in debugfs was not being read or
written correctly, resulting in "Invalid argument" errors.

Fixed this by ensuring proper mapping of inactivity_cp in
both the map_cc_config_offset_gen0_ext0 and
bnxt_re_fill_gen0_ext0() functions.

Fixes: 656dff55da19 ("RDMA/bnxt_re: Congestion control settings using debugfs hook")
Signed-off-by: Gautam R A <gautam-r.a@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/debugfs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
index af91d16c3c77..a3aad6c3dbec 100644
--- a/drivers/infiniband/hw/bnxt_re/debugfs.c
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -170,6 +170,9 @@ static int map_cc_config_offset_gen0_ext0(u32 offset, struct bnxt_qplib_cc_param
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_TCP_CP:
 		*val =  ccparam->tcp_cp;
 		break;
+	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_INACTIVITY_CP:
+		*val = ccparam->inact_th;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -247,7 +250,9 @@ static void bnxt_re_fill_gen0_ext0(struct bnxt_qplib_cc_param *ccparam, u32 offs
 		ccparam->tcp_cp = val;
 		break;
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_TX_QUEUE:
+		break;
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_INACTIVITY_CP:
+		ccparam->inact_th = val;
 		break;
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_TIME_PER_PHASE:
 		ccparam->time_pph = val;
-- 
2.43.5

[PATCH rdma-rc 2/4] RDMA/bnxt_re: Fix missing error handling for tx_queue

[Kalesh AP]

From: Gautam R A <gautam-r.a@broadcom.com>

bnxt_re_fill_gen0_ext0() did not return an error when
attempting to modify CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_TX_QUEUE,
leading to silent failures.

Fixed this by returning -EOPNOTSUPP for tx_queue modifications and
ensuring proper error propagation in bnxt_re_configure_cc().

Fixes: 656dff55da19 ("RDMA/bnxt_re: Congestion control settings using debugfs hook")
Signed-off-by: Gautam R A <gautam-r.a@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/debugfs.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
index a3aad6c3dbec..bd5343406876 100644
--- a/drivers/infiniband/hw/bnxt_re/debugfs.c
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -206,7 +206,7 @@ static ssize_t bnxt_re_cc_config_get(struct file *filp, char __user *buffer,
 	return simple_read_from_buffer(buffer, usr_buf_len, ppos, (u8 *)(buf), rc);
 }
 
-static void bnxt_re_fill_gen0_ext0(struct bnxt_qplib_cc_param *ccparam, u32 offset, u32 val)
+static int bnxt_re_fill_gen0_ext0(struct bnxt_qplib_cc_param *ccparam, u32 offset, u32 val)
 {
 	u32 modify_mask;
 
@@ -250,7 +250,7 @@ static void bnxt_re_fill_gen0_ext0(struct bnxt_qplib_cc_param *ccparam, u32 offs
 		ccparam->tcp_cp = val;
 		break;
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_TX_QUEUE:
-		break;
+		return -EOPNOTSUPP;
 	case CMDQ_MODIFY_ROCE_CC_MODIFY_MASK_INACTIVITY_CP:
 		ccparam->inact_th = val;
 		break;
@@ -263,17 +263,22 @@ static void bnxt_re_fill_gen0_ext0(struct bnxt_qplib_cc_param *ccparam, u32 offs
 	}
 
 	ccparam->mask = modify_mask;
+	return 0;
 }
 
 static int bnxt_re_configure_cc(struct bnxt_re_dev *rdev, u32 gen_ext, u32 offset, u32 val)
 {
 	struct bnxt_qplib_cc_param ccparam = { };
+	int rc;
 
 	/* Supporting only Gen 0 now */
-	if (gen_ext == CC_CONFIG_GEN0_EXT0)
-		bnxt_re_fill_gen0_ext0(&ccparam, offset, val);
-	else
+	if (gen_ext == CC_CONFIG_GEN0_EXT0) {
+		rc = bnxt_re_fill_gen0_ext0(&ccparam, offset, val);
+		if (rc)
+			return rc;
+	} else {
 		return -EINVAL;
+	}
 
 	bnxt_qplib_modify_cc(&rdev->qplib_res, &ccparam);
 	return 0;
-- 
2.43.5

[PATCH rdma-rc 3/4] RDMA/bnxt_re: Fix return code of bnxt_re_configure_cc

[Kalesh AP]

Driver currently supports modifying GEN0_EXT0 CC parameters
through debugfs hook.

Fixed to return -EOPNOTSUPP instead of -EINVAL in bnxt_re_configure_cc()
when the user tries to modify any other CC parameters.

Fixes: 656dff55da19 ("RDMA/bnxt_re: Congestion control settings using debugfs hook")
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
index bd5343406876..4be19e0abd32 100644
--- a/drivers/infiniband/hw/bnxt_re/debugfs.c
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -277,7 +277,7 @@ static int bnxt_re_configure_cc(struct bnxt_re_dev *rdev, u32 gen_ext, u32 offse
 		if (rc)
 			return rc;
 	} else {
-		return -EINVAL;
+		return -EOPNOTSUPP;
 	}
 
 	bnxt_qplib_modify_cc(&rdev->qplib_res, &ccparam);
-- 
2.43.5

[PATCH rdma-rc 4/4] RDMA/bnxt_re: Fix null pointer dereference in bnxt_re_suspend

[Kalesh AP]

There is a possibility that the Ethernet driver invokes bnxt_ulp_stop()
twice during reset recovery scenario. This will result in a NULL pointer
dereference in bnxt_re_suspend() as en_info->rdev has been set to NULL
during the first invocation.

Fixes: dee3da3422d5 ("RDMA/bnxt_re: Change aux driver data to en_info to hold more information")
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 293b0a96c8e3..bc0b40073461 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -2410,6 +2410,8 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
 	struct bnxt_re_dev *rdev;
 
 	rdev = en_info->rdev;
+	if (!rdev)
+		return 0;
 	en_dev = en_info->en_dev;
 	mutex_lock(&bnxt_re_mutex);
 
-- 
2.43.5

Re: [PATCH rdma-rc 4/4] RDMA/bnxt_re: Fix null pointer dereference in bnxt_re_suspend

[Leon Romanovsky]

On Tue, May 20, 2025 at 09:29:10AM +0530, Kalesh AP wrote:
> There is a possibility that the Ethernet driver invokes bnxt_ulp_stop()
> twice during reset recovery scenario. This will result in a NULL pointer
> dereference in bnxt_re_suspend() as en_info->rdev has been set to NULL
> during the first invocation.

Please fix bnxt_ulp_stop() to do not do that. The fix should be there
and not in bnxt_re.

Thanks

> 
> Fixes: dee3da3422d5 ("RDMA/bnxt_re: Change aux driver data to en_info to hold more information")
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> ---
>  drivers/infiniband/hw/bnxt_re/main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 293b0a96c8e3..bc0b40073461 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -2410,6 +2410,8 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
>  	struct bnxt_re_dev *rdev;
>  
>  	rdev = en_info->rdev;
> +	if (!rdev)
> +		return 0;
>  	en_dev = en_info->en_dev;
>  	mutex_lock(&bnxt_re_mutex);
>  
> -- 
> 2.43.5
> 
> 

Re: [PATCH rdma-rc 0/4] bnxt_re bug fixes

[Leon Romanovsky]

On Tue, May 20, 2025 at 09:29:06AM +0530, Kalesh AP wrote:
> This series contains bug fixes related to debugfs
> support in the driver. One fix is for preventing
> a NULL pointer dereference in reset recovery path.
> 
> Please review and apply.
> 
> Regards,
> Kalesh AP
> 
> Gautam R A (2):
>   RDMA/bnxt_re: Fix incorrect display of inactivity_cp in debugfs output
>   RDMA/bnxt_re: Fix missing error handling for tx_queue
> 
> Kalesh AP (2):
>   RDMA/bnxt_re: Fix return code of bnxt_re_configure_cc

Applied to -next.

>   RDMA/bnxt_re: Fix null pointer dereference in bnxt_re_suspend

This bug needs to be fixed in bnxt core driver.

Thanks

> 
>  drivers/infiniband/hw/bnxt_re/debugfs.c | 20 +++++++++++++++-----
>  drivers/infiniband/hw/bnxt_re/main.c    |  2 ++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> -- 
> 2.43.5
> 

Re: [PATCH rdma-rc 4/4] RDMA/bnxt_re: Fix null pointer dereference in bnxt_re_suspend

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]

Hi Leon,

On Wed, May 21, 2025 at 3:31 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, May 20, 2025 at 09:29:10AM +0530, Kalesh AP wrote:
> > There is a possibility that the Ethernet driver invokes bnxt_ulp_stop()
> > twice during reset recovery scenario. This will result in a NULL pointer
> > dereference in bnxt_re_suspend() as en_info->rdev has been set to NULL
> > during the first invocation.
>
> Please fix bnxt_ulp_stop() to do not do that. The fix should be there
> and not in bnxt_re.

Thank you for the review and feedback. Sure, let me see if we can fix
it in the core bnxt driver.
>
> Thanks
>
> >
> > Fixes: dee3da3422d5 ("RDMA/bnxt_re: Change aux driver data to en_info to hold more information")
> > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> > ---
> >  drivers/infiniband/hw/bnxt_re/main.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> > index 293b0a96c8e3..bc0b40073461 100644
> > --- a/drivers/infiniband/hw/bnxt_re/main.c
> > +++ b/drivers/infiniband/hw/bnxt_re/main.c
> > @@ -2410,6 +2410,8 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state)
> >       struct bnxt_re_dev *rdev;
> >
> >       rdev = en_info->rdev;
> > +     if (!rdev)
> > +             return 0;
> >       en_dev = en_info->en_dev;
> >       mutex_lock(&bnxt_re_mutex);
> >
> > --
> > 2.43.5
> >
> >



-- 
Regards,
Kalesh AP

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4226 bytes --]