[dpdk-dev] net/bnxt: fix an erorr with vnic_tpa_cfg command

Message ID 20180228221236.28288-1-ajit.khaparde@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ajit Khaparde Feb. 28, 2018, 10:12 p.m. UTC
  When the vnic_tpa_cfg HWRM command is sent to the FW,
we are not passing the VNIC ID in case of disable.
This can cause the FW to return an error.
Correct VNIC ID needs to be passed for both enable and disable.

Fixes: 0958d8b6435d ("net/bnxt: support LRO")
Cc: stable@dpdk.org

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Ferruh Yigit March 26, 2018, 8:20 p.m. UTC | #1
On 2/28/2018 10:12 PM, Ajit Khaparde wrote:
> When the vnic_tpa_cfg HWRM command is sent to the FW,
> we are not passing the VNIC ID in case of disable.
> This can cause the FW to return an error.
> Correct VNIC ID needs to be passed for both enable and disable.

Hi Ajit,

Patch title doesn't tell what is actually fixed, after reading commit log, will
you be agree on following:

"net/bnxt: fix LRO disable"

> 
> Fixes: 0958d8b6435d ("net/bnxt: support LRO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index b7843afe6..05663fedd 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -1517,12 +1517,12 @@ int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
>  				HWRM_VNIC_TPA_CFG_INPUT_FLAGS_GRO |
>  				HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_ECN |
>  			HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_SAME_GRE_SEQ);
> -		req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
>  		req.max_agg_segs = rte_cpu_to_le_16(5);
>  		req.max_aggs =
>  			rte_cpu_to_le_16(HWRM_VNIC_TPA_CFG_INPUT_MAX_AGGS_MAX);
>  		req.min_agg_len = rte_cpu_to_le_32(512);
>  	}
> +	req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
>  
>  	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req));
>  
>
  
Ajit Khaparde March 26, 2018, 8:25 p.m. UTC | #2
On Mon, Mar 26, 2018 at 1:20 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 2/28/2018 10:12 PM, Ajit Khaparde wrote:
> > When the vnic_tpa_cfg HWRM command is sent to the FW,
> > we are not passing the VNIC ID in case of disable.
> > This can cause the FW to return an error.
> > Correct VNIC ID needs to be passed for both enable and disable.
>
> Hi Ajit,
>
> Patch title doesn't tell what is actually fixed, after reading commit log,
> will
> you be agree on following:
>
> "net/bnxt: fix LRO disable"
>
​Yes, Ferruh. This is fine.​ Thanks



>
> >
> > Fixes: 0958d8b6435d ("net/bnxt: support LRO")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >  drivers/net/bnxt/bnxt_hwrm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> > index b7843afe6..05663fedd 100644
> > --- a/drivers/net/bnxt/bnxt_hwrm.c
> > +++ b/drivers/net/bnxt/bnxt_hwrm.c
> > @@ -1517,12 +1517,12 @@ int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
> >                               HWRM_VNIC_TPA_CFG_INPUT_FLAGS_GRO |
> >                               HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_ECN
> |
> >                       HWRM_VNIC_TPA_CFG_INPUT_FLAGS_
> AGG_WITH_SAME_GRE_SEQ);
> > -             req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
> >               req.max_agg_segs = rte_cpu_to_le_16(5);
> >               req.max_aggs =
> >                       rte_cpu_to_le_16(HWRM_VNIC_
> TPA_CFG_INPUT_MAX_AGGS_MAX);
> >               req.min_agg_len = rte_cpu_to_le_32(512);
> >       }
> > +     req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
> >
> >       rc = bnxt_hwrm_send_message(bp, &req, sizeof(req));
> >
> >
>
>
  
Ferruh Yigit March 26, 2018, 8:33 p.m. UTC | #3
On 2/28/2018 10:12 PM, Ajit Khaparde wrote:
> When the vnic_tpa_cfg HWRM command is sent to the FW,
> we are not passing the VNIC ID in case of disable.
> This can cause the FW to return an error.
> Correct VNIC ID needs to be passed for both enable and disable.
> 
> Fixes: 0958d8b6435d ("net/bnxt: support LRO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index b7843afe6..05663fedd 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1517,12 +1517,12 @@  int bnxt_hwrm_vnic_tpa_cfg(struct bnxt *bp,
 				HWRM_VNIC_TPA_CFG_INPUT_FLAGS_GRO |
 				HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_ECN |
 			HWRM_VNIC_TPA_CFG_INPUT_FLAGS_AGG_WITH_SAME_GRE_SEQ);
-		req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
 		req.max_agg_segs = rte_cpu_to_le_16(5);
 		req.max_aggs =
 			rte_cpu_to_le_16(HWRM_VNIC_TPA_CFG_INPUT_MAX_AGGS_MAX);
 		req.min_agg_len = rte_cpu_to_le_32(512);
 	}
+	req.vnic_id = rte_cpu_to_le_32(vnic->fw_vnic_id);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req));