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

[Wojciech Drewek]

On 26.10.2023 12:37, Ratheesh Kannoth wrote:
>> From: Wojciech Drewek <wojciech.drewek@xxxxxxxxx>
>> 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@xxxxxxxxx>
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,
>>>  };
>>>