[net] octeontx2-pf: Fix holes in error code

Message ID 20231026030154.1317011-1-rkannoth@marvell.com
State New
Headers
Series [net] octeontx2-pf: Fix holes in error code |

Commit Message

Ratheesh Kannoth Oct. 26, 2023, 3:01 a.m. UTC
  Error code strings are not getting printed properly
due to holes. Print error code as well.

Fixes: 51afe9026d0c ("octeontx2-pf: NIX TX overwrites SQ_CTX_HW_S[SQ_INT]")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  | 65 ++++++++++---------
 .../marvell/octeontx2/nic/otx2_struct.h       | 34 +++++-----
 2 files changed, 51 insertions(+), 48 deletions(-)
  

Comments

Wojciech Drewek Oct. 26, 2023, 10:14 a.m. UTC | #1
On 26.10.2023 05:01, Ratheesh Kannoth wrote:
> Error code strings are not getting printed properly
> due to holes. Print error code as well.
> 
> Fixes: 51afe9026d0c ("octeontx2-pf: NIX TX overwrites SQ_CTX_HW_S[SQ_INT]")
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>  .../ethernet/marvell/octeontx2/nic/otx2_pf.c  | 65 ++++++++++---------
>  .../marvell/octeontx2/nic/otx2_struct.h       | 34 +++++-----
>  2 files changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index 6daf4d58c25d..e82724f69406 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -1193,31 +1193,32 @@ static char *nix_mnqerr_e_str[NIX_MNQERR_MAX] = {
>  };
>  
>  static char *nix_snd_status_e_str[NIX_SND_STATUS_MAX] =  {
> -	"NIX_SND_STATUS_GOOD",
> -	"NIX_SND_STATUS_SQ_CTX_FAULT",
> -	"NIX_SND_STATUS_SQ_CTX_POISON",
> -	"NIX_SND_STATUS_SQB_FAULT",
> -	"NIX_SND_STATUS_SQB_POISON",
> -	"NIX_SND_STATUS_HDR_ERR",
> -	"NIX_SND_STATUS_EXT_ERR",
> -	"NIX_SND_STATUS_JUMP_FAULT",
> -	"NIX_SND_STATUS_JUMP_POISON",
> -	"NIX_SND_STATUS_CRC_ERR",
> -	"NIX_SND_STATUS_IMM_ERR",
> -	"NIX_SND_STATUS_SG_ERR",
> -	"NIX_SND_STATUS_MEM_ERR",
> -	"NIX_SND_STATUS_INVALID_SUBDC",
> -	"NIX_SND_STATUS_SUBDC_ORDER_ERR",
> -	"NIX_SND_STATUS_DATA_FAULT",
> -	"NIX_SND_STATUS_DATA_POISON",
> -	"NIX_SND_STATUS_NPC_DROP_ACTION",
> -	"NIX_SND_STATUS_LOCK_VIOL",
> -	"NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
> -	"NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
> -	"NIX_SND_STATUS_NPC_MCAST_ABORT",
> -	"NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
> -	"NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
> -	"NIX_SND_STATUS_SEND_STATS_ERR",
> +	[NIX_SND_STATUS_GOOD] = "NIX_SND_STATUS_GOOD",
> +	[NIX_SND_STATUS_SQ_CTX_FAULT] = "NIX_SND_STATUS_SQ_CTX_FAULT",
> +	[NIX_SND_STATUS_SQ_CTX_POISON] = "NIX_SND_STATUS_SQ_CTX_POISON",
> +	[NIX_SND_STATUS_SQB_FAULT] = "NIX_SND_STATUS_SQB_FAULT",
> +	[NIX_SND_STATUS_SQB_POISON] = "NIX_SND_STATUS_SQB_POISON",
> +	[NIX_SND_STATUS_HDR_ERR] = "NIX_SND_STATUS_HDR_ERR",
> +	[NIX_SND_STATUS_EXT_ERR] = "NIX_SND_STATUS_EXT_ERR",
> +	[NIX_SND_STATUS_JUMP_FAULT] = "NIX_SND_STATUS_JUMP_FAULT",
> +	[NIX_SND_STATUS_JUMP_POISON] = "NIX_SND_STATUS_JUMP_POISON",
> +	[NIX_SND_STATUS_CRC_ERR] = "NIX_SND_STATUS_CRC_ERR",
> +	[NIX_SND_STATUS_IMM_ERR] = "NIX_SND_STATUS_IMM_ERR",
> +	[NIX_SND_STATUS_SG_ERR] = "NIX_SND_STATUS_SG_ERR",
> +	[NIX_SND_STATUS_MEM_ERR] = "NIX_SND_STATUS_MEM_ERR",
> +	[NIX_SND_STATUS_INVALID_SUBDC] = "NIX_SND_STATUS_INVALID_SUBDC",
> +	[NIX_SND_STATUS_SUBDC_ORDER_ERR] = "NIX_SND_STATUS_SUBDC_ORDER_ERR",
> +	[NIX_SND_STATUS_DATA_FAULT] = "NIX_SND_STATUS_DATA_FAULT",
> +	[NIX_SND_STATUS_DATA_POISON] = "NIX_SND_STATUS_DATA_POISON",
> +	[NIX_SND_STATUS_NPC_DROP_ACTION] = "NIX_SND_STATUS_NPC_DROP_ACTION",
> +	[NIX_SND_STATUS_LOCK_VIOL] = "NIX_SND_STATUS_LOCK_VIOL",
> +	[NIX_SND_STATUS_NPC_UCAST_CHAN_ERR] = "NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
> +	[NIX_SND_STATUS_NPC_MCAST_CHAN_ERR] = "NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
> +	[NIX_SND_STATUS_NPC_MCAST_ABORT] = "NIX_SND_STATUS_NPC_MCAST_ABORT",
> +	[NIX_SND_STATUS_NPC_VTAG_PTR_ERR] = "NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
> +	[NIX_SND_STATUS_NPC_VTAG_SIZE_ERR] = "NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
> +	[NIX_SND_STATUS_SEND_MEM_FAULT] = "NIX_SND_STATUS_SEND_MEM_FAULT",
> +	[NIX_SND_STATUS_SEND_STATS_ERR] = "NIX_SND_STATUS_SEND_STATS_ERR",
>  };
>  
>  static irqreturn_t otx2_q_intr_handler(int irq, void *data)
> @@ -1282,8 +1283,8 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data)
>  			goto chk_mnq_err_dbg;
>  
>  		sq_op_err_code = FIELD_GET(GENMASK(7, 0), sq_op_err_dbg);
> -		netdev_err(pf->netdev, "SQ%lld: NIX_LF_SQ_OP_ERR_DBG(%llx)  err=%s\n",
> -			   qidx, sq_op_err_dbg, nix_sqoperr_e_str[sq_op_err_code]);
> +		netdev_err(pf->netdev, "SQ%lld: NIX_LF_SQ_OP_ERR_DBG(0x%llx)  err=%s(%#x)\n",
> +			   qidx, sq_op_err_dbg, nix_sqoperr_e_str[sq_op_err_code], sq_op_err_code);
>  
>  		otx2_write64(pf, NIX_LF_SQ_OP_ERR_DBG, BIT_ULL(44));
>  
> @@ -1300,16 +1301,18 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data)
>  			goto chk_snd_err_dbg;
>  
>  		mnq_err_code = FIELD_GET(GENMASK(7, 0), mnq_err_dbg);
> -		netdev_err(pf->netdev, "SQ%lld: NIX_LF_MNQ_ERR_DBG(%llx)  err=%s\n",
> -			   qidx, mnq_err_dbg,  nix_mnqerr_e_str[mnq_err_code]);
> +		netdev_err(pf->netdev, "SQ%lld: NIX_LF_MNQ_ERR_DBG(0x%llx)  err=%s(%#x)\n",
> +			   qidx, mnq_err_dbg,  nix_mnqerr_e_str[mnq_err_code], mnq_err_code);
>  		otx2_write64(pf, NIX_LF_MNQ_ERR_DBG, BIT_ULL(44));
>  
>  chk_snd_err_dbg:
>  		snd_err_dbg = otx2_read64(pf, NIX_LF_SEND_ERR_DBG);
>  		if (snd_err_dbg & BIT(44)) {
>  			snd_err_code = FIELD_GET(GENMASK(7, 0), snd_err_dbg);
> -			netdev_err(pf->netdev, "SQ%lld: NIX_LF_SND_ERR_DBG:0x%llx err=%s\n",
> -				   qidx, snd_err_dbg, nix_snd_status_e_str[snd_err_code]);
> +			netdev_err(pf->netdev,
> +				   "SQ%lld: NIX_LF_SND_ERR_DBG:0x%llx err=%s(%#x)\n",
> +				   qidx, snd_err_dbg, nix_snd_status_e_str[snd_err_code],
> +				   snd_err_code);
>  			otx2_write64(pf, NIX_LF_SEND_ERR_DBG, BIT_ULL(44));
>  		}
>  
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
> index fa37b9f312ca..4e5899d8fa2e 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
> @@ -318,23 +318,23 @@ enum nix_snd_status_e {
>  	NIX_SND_STATUS_EXT_ERR = 0x6,
>  	NIX_SND_STATUS_JUMP_FAULT = 0x7,
>  	NIX_SND_STATUS_JUMP_POISON = 0x8,
> -	NIX_SND_STATUS_CRC_ERR = 0x9,
> -	NIX_SND_STATUS_IMM_ERR = 0x10,
> -	NIX_SND_STATUS_SG_ERR = 0x11,
> -	NIX_SND_STATUS_MEM_ERR = 0x12,
> -	NIX_SND_STATUS_INVALID_SUBDC = 0x13,
> -	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x14,
> -	NIX_SND_STATUS_DATA_FAULT = 0x15,
> -	NIX_SND_STATUS_DATA_POISON = 0x16,
> -	NIX_SND_STATUS_NPC_DROP_ACTION = 0x17,
> -	NIX_SND_STATUS_LOCK_VIOL = 0x18,
> -	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x19,
> -	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x20,
> -	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x21,
> -	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x22,
> -	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x23,
> -	NIX_SND_STATUS_SEND_MEM_FAULT = 0x24,
> -	NIX_SND_STATUS_SEND_STATS_ERR = 0x25,
> +	NIX_SND_STATUS_CRC_ERR = 0x10,> +	NIX_SND_STATUS_IMM_ERR = 0x11,
> +	NIX_SND_STATUS_SG_ERR = 0x12,
> +	NIX_SND_STATUS_MEM_ERR = 0x13,
> +	NIX_SND_STATUS_INVALID_SUBDC = 0x14,
> +	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x15,
> +	NIX_SND_STATUS_DATA_FAULT = 0x16,
> +	NIX_SND_STATUS_DATA_POISON = 0x17,

There is a gap here, is it intended?
And in general, why error codes were changed?
Starting from NIX_SND_STATUS_CRC_ERR which was 0x09 and now it's 0x10.

> +	NIX_SND_STATUS_NPC_DROP_ACTION = 0x20,
> +	NIX_SND_STATUS_LOCK_VIOL = 0x21,
> +	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x22,
> +	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x23,
> +	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x24,
> +	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x25,
> +	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x26,
> +	NIX_SND_STATUS_SEND_MEM_FAULT = 0x27,
> +	NIX_SND_STATUS_SEND_STATS_ERR = 0x28,
>  	NIX_SND_STATUS_MAX,
>  };
>
  
Ratheesh Kannoth Oct. 26, 2023, 10:37 a.m. UTC | #2
> From: Wojciech Drewek <wojciech.drewek@intel.com>
> Sent: Thursday, October 26, 2023 3:44 PM
> Subject: [EXT] Re: [PATCH net] octeontx2-pf: Fix holes in error code
> 
> >  static char *nix_snd_status_e_str[NIX_SND_STATUS_MAX] =  {
> > -	"NIX_SND_STATUS_GOOD",
> > -	"NIX_SND_STATUS_SQ_CTX_FAULT",
> > -	"NIX_SND_STATUS_SQ_CTX_POISON",
> > -	"NIX_SND_STATUS_SQB_FAULT",
> > -	"NIX_SND_STATUS_SQB_POISON",
> > -	"NIX_SND_STATUS_HDR_ERR",
> > -	"NIX_SND_STATUS_EXT_ERR",
> > -	"NIX_SND_STATUS_JUMP_FAULT",
> > -	"NIX_SND_STATUS_JUMP_POISON",
> > -	"NIX_SND_STATUS_CRC_ERR",
> > -	"NIX_SND_STATUS_IMM_ERR",
> > -	"NIX_SND_STATUS_SG_ERR",
> > -	"NIX_SND_STATUS_MEM_ERR",
> > -	"NIX_SND_STATUS_INVALID_SUBDC",
> > -	"NIX_SND_STATUS_SUBDC_ORDER_ERR",
> > -	"NIX_SND_STATUS_DATA_FAULT",
> > -	"NIX_SND_STATUS_DATA_POISON",
> > -	"NIX_SND_STATUS_NPC_DROP_ACTION",
> > -	"NIX_SND_STATUS_LOCK_VIOL",
> > -	"NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
> > -	"NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
> > -	"NIX_SND_STATUS_NPC_MCAST_ABORT",
> > -	"NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
> > -	"NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
> > -	"NIX_SND_STATUS_SEND_STATS_ERR",
> > +	[NIX_SND_STATUS_GOOD] = "NIX_SND_STATUS_GOOD",
> > +	[NIX_SND_STATUS_SQ_CTX_FAULT] =
> "NIX_SND_STATUS_SQ_CTX_FAULT",
> > +	[NIX_SND_STATUS_SQ_CTX_POISON] =
> "NIX_SND_STATUS_SQ_CTX_POISON",
> > +	[NIX_SND_STATUS_SQB_FAULT] = "NIX_SND_STATUS_SQB_FAULT",
> > +	[NIX_SND_STATUS_SQB_POISON] =
> "NIX_SND_STATUS_SQB_POISON",
> > +	[NIX_SND_STATUS_HDR_ERR] = "NIX_SND_STATUS_HDR_ERR",
> > +	[NIX_SND_STATUS_EXT_ERR] = "NIX_SND_STATUS_EXT_ERR",
> > +	[NIX_SND_STATUS_JUMP_FAULT] =
> "NIX_SND_STATUS_JUMP_FAULT",
> > +	[NIX_SND_STATUS_JUMP_POISON] =
> "NIX_SND_STATUS_JUMP_POISON",
> > +	[NIX_SND_STATUS_CRC_ERR] = "NIX_SND_STATUS_CRC_ERR",
> > +	[NIX_SND_STATUS_IMM_ERR] = "NIX_SND_STATUS_IMM_ERR",
> > +	[NIX_SND_STATUS_SG_ERR] = "NIX_SND_STATUS_SG_ERR",
> > +	[NIX_SND_STATUS_MEM_ERR] = "NIX_SND_STATUS_MEM_ERR",
> > +	[NIX_SND_STATUS_INVALID_SUBDC] =
> "NIX_SND_STATUS_INVALID_SUBDC",
> > +	[NIX_SND_STATUS_SUBDC_ORDER_ERR] =
> "NIX_SND_STATUS_SUBDC_ORDER_ERR",
> > +	[NIX_SND_STATUS_DATA_FAULT] =
> "NIX_SND_STATUS_DATA_FAULT",
> > +	[NIX_SND_STATUS_DATA_POISON] =
> "NIX_SND_STATUS_DATA_POISON",
> > +	[NIX_SND_STATUS_NPC_DROP_ACTION] =
> "NIX_SND_STATUS_NPC_DROP_ACTION",
> > +	[NIX_SND_STATUS_LOCK_VIOL] = "NIX_SND_STATUS_LOCK_VIOL",
> > +	[NIX_SND_STATUS_NPC_UCAST_CHAN_ERR] =
> "NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
> > +	[NIX_SND_STATUS_NPC_MCAST_CHAN_ERR] =
> "NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
> > +	[NIX_SND_STATUS_NPC_MCAST_ABORT] =
> "NIX_SND_STATUS_NPC_MCAST_ABORT",
> > +	[NIX_SND_STATUS_NPC_VTAG_PTR_ERR] =
> "NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
> > +	[NIX_SND_STATUS_NPC_VTAG_SIZE_ERR] =
> "NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
> > +	[NIX_SND_STATUS_SEND_MEM_FAULT] =
> "NIX_SND_STATUS_SEND_MEM_FAULT",
> > +	[NIX_SND_STATUS_SEND_STATS_ERR] =
> "NIX_SND_STATUS_SEND_STATS_ERR",
> >  };
> >
> > @@ -318,23 +318,23 @@ enum nix_snd_status_e {
> >  	NIX_SND_STATUS_EXT_ERR = 0x6,
> >  	NIX_SND_STATUS_JUMP_FAULT = 0x7,
> >  	NIX_SND_STATUS_JUMP_POISON = 0x8,
> > -	NIX_SND_STATUS_CRC_ERR = 0x9,
> > -	NIX_SND_STATUS_IMM_ERR = 0x10,
> > -	NIX_SND_STATUS_SG_ERR = 0x11,
> > -	NIX_SND_STATUS_MEM_ERR = 0x12,
> > -	NIX_SND_STATUS_INVALID_SUBDC = 0x13,
> > -	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x14,
> > -	NIX_SND_STATUS_DATA_FAULT = 0x15,
> > -	NIX_SND_STATUS_DATA_POISON = 0x16,
> > -	NIX_SND_STATUS_NPC_DROP_ACTION = 0x17,
> > -	NIX_SND_STATUS_LOCK_VIOL = 0x18,
> > -	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x19,
> > -	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x20,
> > -	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x21,
> > -	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x22,
> > -	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x23,
> > -	NIX_SND_STATUS_SEND_MEM_FAULT = 0x24,
> > -	NIX_SND_STATUS_SEND_STATS_ERR = 0x25,
> > +	NIX_SND_STATUS_CRC_ERR = 0x10,> +
> 	NIX_SND_STATUS_IMM_ERR = 0x11,
> > +	NIX_SND_STATUS_SG_ERR = 0x12,
> > +	NIX_SND_STATUS_MEM_ERR = 0x13,
> > +	NIX_SND_STATUS_INVALID_SUBDC = 0x14,
> > +	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x15,
> > +	NIX_SND_STATUS_DATA_FAULT = 0x16,
> > +	NIX_SND_STATUS_DATA_POISON = 0x17,
> 
> There is a gap here, is it intended?
[Ratheesh Kannoth] Yes.

> And in general, why error codes were changed?
[Ratheesh Kannoth] it was a bug. 
 
> Starting from NIX_SND_STATUS_CRC_ERR which was 0x09 and now it's 0x10.
[Ratheesh Kannoth] Yes. It should be 0x10 .  The real issue is - >enum was not in sequence.  (Eg ,,after 0x9 it should be 0xa).  But 
"static char *nix_snd_status_e_str" was assigned sequential strings.  

> 
> > +	NIX_SND_STATUS_NPC_DROP_ACTION = 0x20,
> > +	NIX_SND_STATUS_LOCK_VIOL = 0x21,
> > +	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x22,
> > +	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x23,
> > +	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x24,
> > +	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x25,
> > +	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x26,
> > +	NIX_SND_STATUS_SEND_MEM_FAULT = 0x27,
> > +	NIX_SND_STATUS_SEND_STATS_ERR = 0x28,
> >  	NIX_SND_STATUS_MAX,
> >  };
> >
  
Wojciech Drewek Oct. 26, 2023, 10:56 a.m. UTC | #3
On 26.10.2023 12:37, Ratheesh Kannoth wrote:
>> From: Wojciech Drewek <wojciech.drewek@intel.com>
>> Sent: Thursday, October 26, 2023 3:44 PM
>> Subject: [EXT] Re: [PATCH net] octeontx2-pf: Fix holes in error code
>>
>>>  static char *nix_snd_status_e_str[NIX_SND_STATUS_MAX] =  {
>>> -	"NIX_SND_STATUS_GOOD",
>>> -	"NIX_SND_STATUS_SQ_CTX_FAULT",
>>> -	"NIX_SND_STATUS_SQ_CTX_POISON",
>>> -	"NIX_SND_STATUS_SQB_FAULT",
>>> -	"NIX_SND_STATUS_SQB_POISON",
>>> -	"NIX_SND_STATUS_HDR_ERR",
>>> -	"NIX_SND_STATUS_EXT_ERR",
>>> -	"NIX_SND_STATUS_JUMP_FAULT",
>>> -	"NIX_SND_STATUS_JUMP_POISON",
>>> -	"NIX_SND_STATUS_CRC_ERR",
>>> -	"NIX_SND_STATUS_IMM_ERR",
>>> -	"NIX_SND_STATUS_SG_ERR",
>>> -	"NIX_SND_STATUS_MEM_ERR",
>>> -	"NIX_SND_STATUS_INVALID_SUBDC",
>>> -	"NIX_SND_STATUS_SUBDC_ORDER_ERR",
>>> -	"NIX_SND_STATUS_DATA_FAULT",
>>> -	"NIX_SND_STATUS_DATA_POISON",
>>> -	"NIX_SND_STATUS_NPC_DROP_ACTION",
>>> -	"NIX_SND_STATUS_LOCK_VIOL",
>>> -	"NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
>>> -	"NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
>>> -	"NIX_SND_STATUS_NPC_MCAST_ABORT",
>>> -	"NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
>>> -	"NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
>>> -	"NIX_SND_STATUS_SEND_STATS_ERR",
>>> +	[NIX_SND_STATUS_GOOD] = "NIX_SND_STATUS_GOOD",
>>> +	[NIX_SND_STATUS_SQ_CTX_FAULT] =
>> "NIX_SND_STATUS_SQ_CTX_FAULT",
>>> +	[NIX_SND_STATUS_SQ_CTX_POISON] =
>> "NIX_SND_STATUS_SQ_CTX_POISON",
>>> +	[NIX_SND_STATUS_SQB_FAULT] = "NIX_SND_STATUS_SQB_FAULT",
>>> +	[NIX_SND_STATUS_SQB_POISON] =
>> "NIX_SND_STATUS_SQB_POISON",
>>> +	[NIX_SND_STATUS_HDR_ERR] = "NIX_SND_STATUS_HDR_ERR",
>>> +	[NIX_SND_STATUS_EXT_ERR] = "NIX_SND_STATUS_EXT_ERR",
>>> +	[NIX_SND_STATUS_JUMP_FAULT] =
>> "NIX_SND_STATUS_JUMP_FAULT",
>>> +	[NIX_SND_STATUS_JUMP_POISON] =
>> "NIX_SND_STATUS_JUMP_POISON",
>>> +	[NIX_SND_STATUS_CRC_ERR] = "NIX_SND_STATUS_CRC_ERR",
>>> +	[NIX_SND_STATUS_IMM_ERR] = "NIX_SND_STATUS_IMM_ERR",
>>> +	[NIX_SND_STATUS_SG_ERR] = "NIX_SND_STATUS_SG_ERR",
>>> +	[NIX_SND_STATUS_MEM_ERR] = "NIX_SND_STATUS_MEM_ERR",
>>> +	[NIX_SND_STATUS_INVALID_SUBDC] =
>> "NIX_SND_STATUS_INVALID_SUBDC",
>>> +	[NIX_SND_STATUS_SUBDC_ORDER_ERR] =
>> "NIX_SND_STATUS_SUBDC_ORDER_ERR",
>>> +	[NIX_SND_STATUS_DATA_FAULT] =
>> "NIX_SND_STATUS_DATA_FAULT",
>>> +	[NIX_SND_STATUS_DATA_POISON] =
>> "NIX_SND_STATUS_DATA_POISON",
>>> +	[NIX_SND_STATUS_NPC_DROP_ACTION] =
>> "NIX_SND_STATUS_NPC_DROP_ACTION",
>>> +	[NIX_SND_STATUS_LOCK_VIOL] = "NIX_SND_STATUS_LOCK_VIOL",
>>> +	[NIX_SND_STATUS_NPC_UCAST_CHAN_ERR] =
>> "NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
>>> +	[NIX_SND_STATUS_NPC_MCAST_CHAN_ERR] =
>> "NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
>>> +	[NIX_SND_STATUS_NPC_MCAST_ABORT] =
>> "NIX_SND_STATUS_NPC_MCAST_ABORT",
>>> +	[NIX_SND_STATUS_NPC_VTAG_PTR_ERR] =
>> "NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
>>> +	[NIX_SND_STATUS_NPC_VTAG_SIZE_ERR] =
>> "NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
>>> +	[NIX_SND_STATUS_SEND_MEM_FAULT] =
>> "NIX_SND_STATUS_SEND_MEM_FAULT",
>>> +	[NIX_SND_STATUS_SEND_STATS_ERR] =
>> "NIX_SND_STATUS_SEND_STATS_ERR",
>>>  };
>>>
>>> @@ -318,23 +318,23 @@ enum nix_snd_status_e {
>>>  	NIX_SND_STATUS_EXT_ERR = 0x6,
>>>  	NIX_SND_STATUS_JUMP_FAULT = 0x7,
>>>  	NIX_SND_STATUS_JUMP_POISON = 0x8,
>>> -	NIX_SND_STATUS_CRC_ERR = 0x9,
>>> -	NIX_SND_STATUS_IMM_ERR = 0x10,
>>> -	NIX_SND_STATUS_SG_ERR = 0x11,
>>> -	NIX_SND_STATUS_MEM_ERR = 0x12,
>>> -	NIX_SND_STATUS_INVALID_SUBDC = 0x13,
>>> -	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x14,
>>> -	NIX_SND_STATUS_DATA_FAULT = 0x15,
>>> -	NIX_SND_STATUS_DATA_POISON = 0x16,
>>> -	NIX_SND_STATUS_NPC_DROP_ACTION = 0x17,
>>> -	NIX_SND_STATUS_LOCK_VIOL = 0x18,
>>> -	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x19,
>>> -	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x20,
>>> -	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x21,
>>> -	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x22,
>>> -	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x23,
>>> -	NIX_SND_STATUS_SEND_MEM_FAULT = 0x24,
>>> -	NIX_SND_STATUS_SEND_STATS_ERR = 0x25,
>>> +	NIX_SND_STATUS_CRC_ERR = 0x10,> +
>> 	NIX_SND_STATUS_IMM_ERR = 0x11,
>>> +	NIX_SND_STATUS_SG_ERR = 0x12,
>>> +	NIX_SND_STATUS_MEM_ERR = 0x13,
>>> +	NIX_SND_STATUS_INVALID_SUBDC = 0x14,
>>> +	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x15,
>>> +	NIX_SND_STATUS_DATA_FAULT = 0x16,
>>> +	NIX_SND_STATUS_DATA_POISON = 0x17,
>>
>> There is a gap here, is it intended?
> [Ratheesh Kannoth] Yes.
> 
>> And in general, why error codes were changed?
> [Ratheesh Kannoth] it was a bug. 
>  
>> Starting from NIX_SND_STATUS_CRC_ERR which was 0x09 and now it's 0x10.
> [Ratheesh Kannoth] Yes. It should be 0x10 .  The real issue is - >enum was not in sequence.  (Eg ,,after 0x9 it should be 0xa).  But 
> "static char *nix_snd_status_e_str" was assigned sequential strings.  

Ok, got it now.
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
I'd add to the commit msg that some error values were wrong and it it was fixed, but it's a nit.

> 
>>
>>> +	NIX_SND_STATUS_NPC_DROP_ACTION = 0x20,
>>> +	NIX_SND_STATUS_LOCK_VIOL = 0x21,
>>> +	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x22,
>>> +	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x23,
>>> +	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x24,
>>> +	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x25,
>>> +	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x26,
>>> +	NIX_SND_STATUS_SEND_MEM_FAULT = 0x27,
>>> +	NIX_SND_STATUS_SEND_STATS_ERR = 0x28,
>>>  	NIX_SND_STATUS_MAX,
>>>  };
>>>
  
Jakub Kicinski Oct. 26, 2023, 9:52 p.m. UTC | #4
On Thu, 26 Oct 2023 12:56:26 +0200 Wojciech Drewek wrote:
> I'd add to the commit msg that some error values were wrong and it it
> was fixed, but it's a nit.

Agreed, it should be explained in the commit message.
Borderline it deserves to be a separate fix.
  
Ratheesh Kannoth Oct. 27, 2023, 2:01 a.m. UTC | #5
> From: Jakub Kicinski <kuba@kernel.org>
> Subject: Re: [EXT] Re: [PATCH net] octeontx2-pf: Fix holes in error code
> 
> On Thu, 26 Oct 2023 12:56:26 +0200 Wojciech Drewek wrote:
> > I'd add to the commit msg that some error values were wrong and it it
> > was fixed, but it's a nit.
> 
> Agreed, it should be explained in the commit message.
> Borderline it deserves to be a separate fix.
ACK.
  

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 6daf4d58c25d..e82724f69406 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -1193,31 +1193,32 @@  static char *nix_mnqerr_e_str[NIX_MNQERR_MAX] = {
 };
 
 static char *nix_snd_status_e_str[NIX_SND_STATUS_MAX] =  {
-	"NIX_SND_STATUS_GOOD",
-	"NIX_SND_STATUS_SQ_CTX_FAULT",
-	"NIX_SND_STATUS_SQ_CTX_POISON",
-	"NIX_SND_STATUS_SQB_FAULT",
-	"NIX_SND_STATUS_SQB_POISON",
-	"NIX_SND_STATUS_HDR_ERR",
-	"NIX_SND_STATUS_EXT_ERR",
-	"NIX_SND_STATUS_JUMP_FAULT",
-	"NIX_SND_STATUS_JUMP_POISON",
-	"NIX_SND_STATUS_CRC_ERR",
-	"NIX_SND_STATUS_IMM_ERR",
-	"NIX_SND_STATUS_SG_ERR",
-	"NIX_SND_STATUS_MEM_ERR",
-	"NIX_SND_STATUS_INVALID_SUBDC",
-	"NIX_SND_STATUS_SUBDC_ORDER_ERR",
-	"NIX_SND_STATUS_DATA_FAULT",
-	"NIX_SND_STATUS_DATA_POISON",
-	"NIX_SND_STATUS_NPC_DROP_ACTION",
-	"NIX_SND_STATUS_LOCK_VIOL",
-	"NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
-	"NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
-	"NIX_SND_STATUS_NPC_MCAST_ABORT",
-	"NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
-	"NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
-	"NIX_SND_STATUS_SEND_STATS_ERR",
+	[NIX_SND_STATUS_GOOD] = "NIX_SND_STATUS_GOOD",
+	[NIX_SND_STATUS_SQ_CTX_FAULT] = "NIX_SND_STATUS_SQ_CTX_FAULT",
+	[NIX_SND_STATUS_SQ_CTX_POISON] = "NIX_SND_STATUS_SQ_CTX_POISON",
+	[NIX_SND_STATUS_SQB_FAULT] = "NIX_SND_STATUS_SQB_FAULT",
+	[NIX_SND_STATUS_SQB_POISON] = "NIX_SND_STATUS_SQB_POISON",
+	[NIX_SND_STATUS_HDR_ERR] = "NIX_SND_STATUS_HDR_ERR",
+	[NIX_SND_STATUS_EXT_ERR] = "NIX_SND_STATUS_EXT_ERR",
+	[NIX_SND_STATUS_JUMP_FAULT] = "NIX_SND_STATUS_JUMP_FAULT",
+	[NIX_SND_STATUS_JUMP_POISON] = "NIX_SND_STATUS_JUMP_POISON",
+	[NIX_SND_STATUS_CRC_ERR] = "NIX_SND_STATUS_CRC_ERR",
+	[NIX_SND_STATUS_IMM_ERR] = "NIX_SND_STATUS_IMM_ERR",
+	[NIX_SND_STATUS_SG_ERR] = "NIX_SND_STATUS_SG_ERR",
+	[NIX_SND_STATUS_MEM_ERR] = "NIX_SND_STATUS_MEM_ERR",
+	[NIX_SND_STATUS_INVALID_SUBDC] = "NIX_SND_STATUS_INVALID_SUBDC",
+	[NIX_SND_STATUS_SUBDC_ORDER_ERR] = "NIX_SND_STATUS_SUBDC_ORDER_ERR",
+	[NIX_SND_STATUS_DATA_FAULT] = "NIX_SND_STATUS_DATA_FAULT",
+	[NIX_SND_STATUS_DATA_POISON] = "NIX_SND_STATUS_DATA_POISON",
+	[NIX_SND_STATUS_NPC_DROP_ACTION] = "NIX_SND_STATUS_NPC_DROP_ACTION",
+	[NIX_SND_STATUS_LOCK_VIOL] = "NIX_SND_STATUS_LOCK_VIOL",
+	[NIX_SND_STATUS_NPC_UCAST_CHAN_ERR] = "NIX_SND_STATUS_NPC_UCAST_CHAN_ERR",
+	[NIX_SND_STATUS_NPC_MCAST_CHAN_ERR] = "NIX_SND_STATUS_NPC_MCAST_CHAN_ERR",
+	[NIX_SND_STATUS_NPC_MCAST_ABORT] = "NIX_SND_STATUS_NPC_MCAST_ABORT",
+	[NIX_SND_STATUS_NPC_VTAG_PTR_ERR] = "NIX_SND_STATUS_NPC_VTAG_PTR_ERR",
+	[NIX_SND_STATUS_NPC_VTAG_SIZE_ERR] = "NIX_SND_STATUS_NPC_VTAG_SIZE_ERR",
+	[NIX_SND_STATUS_SEND_MEM_FAULT] = "NIX_SND_STATUS_SEND_MEM_FAULT",
+	[NIX_SND_STATUS_SEND_STATS_ERR] = "NIX_SND_STATUS_SEND_STATS_ERR",
 };
 
 static irqreturn_t otx2_q_intr_handler(int irq, void *data)
@@ -1282,8 +1283,8 @@  static irqreturn_t otx2_q_intr_handler(int irq, void *data)
 			goto chk_mnq_err_dbg;
 
 		sq_op_err_code = FIELD_GET(GENMASK(7, 0), sq_op_err_dbg);
-		netdev_err(pf->netdev, "SQ%lld: NIX_LF_SQ_OP_ERR_DBG(%llx)  err=%s\n",
-			   qidx, sq_op_err_dbg, nix_sqoperr_e_str[sq_op_err_code]);
+		netdev_err(pf->netdev, "SQ%lld: NIX_LF_SQ_OP_ERR_DBG(0x%llx)  err=%s(%#x)\n",
+			   qidx, sq_op_err_dbg, nix_sqoperr_e_str[sq_op_err_code], sq_op_err_code);
 
 		otx2_write64(pf, NIX_LF_SQ_OP_ERR_DBG, BIT_ULL(44));
 
@@ -1300,16 +1301,18 @@  static irqreturn_t otx2_q_intr_handler(int irq, void *data)
 			goto chk_snd_err_dbg;
 
 		mnq_err_code = FIELD_GET(GENMASK(7, 0), mnq_err_dbg);
-		netdev_err(pf->netdev, "SQ%lld: NIX_LF_MNQ_ERR_DBG(%llx)  err=%s\n",
-			   qidx, mnq_err_dbg,  nix_mnqerr_e_str[mnq_err_code]);
+		netdev_err(pf->netdev, "SQ%lld: NIX_LF_MNQ_ERR_DBG(0x%llx)  err=%s(%#x)\n",
+			   qidx, mnq_err_dbg,  nix_mnqerr_e_str[mnq_err_code], mnq_err_code);
 		otx2_write64(pf, NIX_LF_MNQ_ERR_DBG, BIT_ULL(44));
 
 chk_snd_err_dbg:
 		snd_err_dbg = otx2_read64(pf, NIX_LF_SEND_ERR_DBG);
 		if (snd_err_dbg & BIT(44)) {
 			snd_err_code = FIELD_GET(GENMASK(7, 0), snd_err_dbg);
-			netdev_err(pf->netdev, "SQ%lld: NIX_LF_SND_ERR_DBG:0x%llx err=%s\n",
-				   qidx, snd_err_dbg, nix_snd_status_e_str[snd_err_code]);
+			netdev_err(pf->netdev,
+				   "SQ%lld: NIX_LF_SND_ERR_DBG:0x%llx err=%s(%#x)\n",
+				   qidx, snd_err_dbg, nix_snd_status_e_str[snd_err_code],
+				   snd_err_code);
 			otx2_write64(pf, NIX_LF_SEND_ERR_DBG, BIT_ULL(44));
 		}
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
index fa37b9f312ca..4e5899d8fa2e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_struct.h
@@ -318,23 +318,23 @@  enum nix_snd_status_e {
 	NIX_SND_STATUS_EXT_ERR = 0x6,
 	NIX_SND_STATUS_JUMP_FAULT = 0x7,
 	NIX_SND_STATUS_JUMP_POISON = 0x8,
-	NIX_SND_STATUS_CRC_ERR = 0x9,
-	NIX_SND_STATUS_IMM_ERR = 0x10,
-	NIX_SND_STATUS_SG_ERR = 0x11,
-	NIX_SND_STATUS_MEM_ERR = 0x12,
-	NIX_SND_STATUS_INVALID_SUBDC = 0x13,
-	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x14,
-	NIX_SND_STATUS_DATA_FAULT = 0x15,
-	NIX_SND_STATUS_DATA_POISON = 0x16,
-	NIX_SND_STATUS_NPC_DROP_ACTION = 0x17,
-	NIX_SND_STATUS_LOCK_VIOL = 0x18,
-	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x19,
-	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x20,
-	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x21,
-	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x22,
-	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x23,
-	NIX_SND_STATUS_SEND_MEM_FAULT = 0x24,
-	NIX_SND_STATUS_SEND_STATS_ERR = 0x25,
+	NIX_SND_STATUS_CRC_ERR = 0x10,
+	NIX_SND_STATUS_IMM_ERR = 0x11,
+	NIX_SND_STATUS_SG_ERR = 0x12,
+	NIX_SND_STATUS_MEM_ERR = 0x13,
+	NIX_SND_STATUS_INVALID_SUBDC = 0x14,
+	NIX_SND_STATUS_SUBDC_ORDER_ERR = 0x15,
+	NIX_SND_STATUS_DATA_FAULT = 0x16,
+	NIX_SND_STATUS_DATA_POISON = 0x17,
+	NIX_SND_STATUS_NPC_DROP_ACTION = 0x20,
+	NIX_SND_STATUS_LOCK_VIOL = 0x21,
+	NIX_SND_STATUS_NPC_UCAST_CHAN_ERR = 0x22,
+	NIX_SND_STATUS_NPC_MCAST_CHAN_ERR = 0x23,
+	NIX_SND_STATUS_NPC_MCAST_ABORT = 0x24,
+	NIX_SND_STATUS_NPC_VTAG_PTR_ERR = 0x25,
+	NIX_SND_STATUS_NPC_VTAG_SIZE_ERR = 0x26,
+	NIX_SND_STATUS_SEND_MEM_FAULT = 0x27,
+	NIX_SND_STATUS_SEND_STATS_ERR = 0x28,
 	NIX_SND_STATUS_MAX,
 };