Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <0d1cb5d7-53e8-45bd-ad45-48fae64b8d9c@intel.com>
Date:   Thu, 26 Oct 2023 12:56:26 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [EXT] Re: [PATCH net] octeontx2-pf: Fix holes in error code
Content-Language: en-US
To:     Ratheesh Kannoth <rkannoth@marvell.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        Hariprasad Kelam <hkelam@marvell.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>
References: <20231026030154.1317011-1-rkannoth@marvell.com>
 <5819e6c8-d887-40ca-9791-07c733126e64@intel.com>
 <MWHPR1801MB1918043F69F45C9D69656449D3DDA@MWHPR1801MB1918.namprd18.prod.outlook.com>
From:   Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <MWHPR1801MB1918043F69F45C9D69656449D3DDA@MWHPR1801MB1918.namprd18.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SE5pRUY0S1JUSWl3ODNISmY2NkpxakhLd1RqSHV2UVBuT0FqakFsWUFTZERZ?=
 =?utf-8?B?cjlIdnIzQVIyVzc4MUVrRGdHVCtjZUhjS1FYUEVKTW5iMmhGaWtUQWkrOVdv?=
 =?utf-8?B?UmJEYWVZakF6dDNoaytKRUlwSEE5Q2doMDRXQ2dGRzdjb1FGdTdwS0ZFczlm?=
 =?utf-8?B?N0ovem9nY0hhb0xXMWNOL1dlQzZKNkh4QkorNFhvdm5GUGhPVjVzSDhKOUFL?=
 =?utf-8?B?cVowdDRIZDZscXJUVWJjMmV3ZDI1ekFJVjk1YUpVc1ROeDREcEMyQVBFSk9W?=
 =?utf-8?B?dW5hQlhERndsMEpVcmN6WWZFeHpFTHpQR1lweDNzdTJSaEpBSHpCQUVzSkxB?=
 =?utf-8?B?NS9hczZscFdpeTFVUmF1SHFlbU5obzlYT2RtMGhpa242UlBjZkwwN3BnUWsz?=
 =?utf-8?B?dytyaUtnZGpFcmRXWTJyMm9sWjJNcVRsSjJ4YmJYcG9tbUdhWU1KNEJMSGpU?=
 =?utf-8?B?MSswMlRPaFZ3U0tPSnBlM0toYlBCeVl3NmhCdENYeVVEZkt0ek4rRnlqQVZk?=
 =?utf-8?B?ZnNJQllVKytxUWVBd2FESUFwQ1F5Rm1mNEpYTGZreHJvUEs5ZVY3RVNxQ0VL?=
 =?utf-8?B?UVIwNm1KaXB5cGNWaEtObkx2djlIbjg4dmtSeGN4djlzaFFiVVhGcTJBNm5u?=
 =?utf-8?B?RUxJS3JQVDZSOWNiTEM1S2txMkR5YUZPYjZyM3dBc0JHcEdKWlJWczlaSEly?=
 =?utf-8?B?ZlZyb2pidjdnYTJxQThNOU04TkJ5SGdpSUNlM1NIUHdtK201dklRZENaUDFL?=
 =?utf-8?B?UC9HUldjYi9OUlVXckl6UTNMWlBJbm4rRFdqU3ZKUERiT0ZxTWd0SEZtNlJq?=
 =?utf-8?B?b2oveUhZbzVVcjlJQ0NzT2oyc2JpTTNUQ3ZHQzZ2SS8vcG1ma251dzd0UEhB?=
 =?utf-8?B?QUNUMWxGWklnVkFmdWVMdUo5eGE2cnZHbE1teWxXNEFiWkcyT1E2Sjh6bVJi?=
 =?utf-8?B?UEl5UTNYTlFVNENQME5qVjR6VWlKcnpVQVlHRDA1Ky9wb3NqV0kxSHB3RzVV?=
 =?utf-8?B?cGNzVFJaZ1lmTUJKeExXK1JMWEdvdGs1WkdUUkZ2dGxTd0NOWlQvbzBVMzYy?=
 =?utf-8?B?Ym1WazZ3TnJXczhSMmxPS0NwNDV6VzAweis0OU9sSXFmMzJBWmJEcmFScFUz?=
 =?utf-8?B?QS8rZnI4WEhiYmtUazhFYzNPYnFoTUFXMmUxK3NwbXVUUzQ0WXlTVlR0emgy?=
 =?utf-8?B?MGE4RjI0eEc4cTc5eEROellncndCT29hc1BoSHY5Ym9oV1FVRHU1SHRWTVIv?=
 =?utf-8?B?SitaZHlBMmpQOTFISnFTOXB2TzdrZkpaWTR3TThVbVRsa21PZFZUWmQ1bGZJ?=
 =?utf-8?B?VmVUWU1aSjVKbjZJMVErT3I1Q0hJZS9mbzdrU0Q0UERocXFaTDB3RTRwUjNa?=
 =?utf-8?B?czNGUnY4L2hYdnIrU1pSaHdHdE02bmh5YSsrUTI0RzBoQmFYNXNsaFU0aEV1?=
 =?utf-8?B?NW5YL3RZWVN5K2FFTFRQUDhYbnJrdmVCbk9QU1k2QTM0UkZVME5kYXBvY2hx?=
 =?utf-8?B?QnNFQXl1ODRoTDNUTnRXamlrTDNDVExlM2JpS0F5aEIwSFRsSTB3YTZ4Qi9w?=
 =?utf-8?B?UWJGcnpUcWdsb0NCdzhmMWxwNGtaUUJkUDRzbTd0QTYxN3FDWFlXc1pUN2R6?=
 =?utf-8?B?R0xWSERKRkRYOHhveE5CWDlOc2ZJc3B6RVd3cEdaS253aE93UnIrSnp5NEFn?=
 =?utf-8?B?aDcrM0VUS3RDcTlPck95cUQ2WElLdUZZSTdQN3U5ay9xS01LcEkxZldxdHM5?=
 =?utf-8?B?L3ZzNzVweU5GRmJocnUzbWp0K3ZHYmEyeUpjcG1YQlJvWG05dzgwYzJySnow?=
 =?utf-8?B?ZDlVTWlaQjBLTmRBRjg1dWtUbnUydldmdXIvNnFvSzBHZHdsNExTVllFYTNv?=
 =?utf-8?B?azRZbko5ckZBSHFDNytPSHIraVoyQzRKSFZza3BsVlU3c3FzUG8yZ2UxQnZK?=
 =?utf-8?B?M3lmQjlhNE1kbWZWSkltUnUwMFREc1pMQkxzanJPV1pUd1k3MVU0bEhGNEhO?=
 =?utf-8?B?TDVRcUYrV0tXRk5vZW94ckVvU2xkcytWMGJlOWRHVjZDRkRidGFsNllSNXp1?=
 =?utf-8?B?dk0xSVd2cDArSmNuZTFtMUhZMHJsR0JXeTZyR21wVUE4YlQvRndYa2RrS1M1?=
 =?utf-8?B?eW9GVzdvTGExRVU5SnlPMnkrWnE3amErVUlpeWNCWEp5TkwwMmZqK1FhdTFS?=
 =?utf-8?B?dHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5e24f480-026a-424c-a01c-08dbd6123749
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 10:56:32.9183
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8RaNfOrrGWJKEYV/HjVnd3dKRrLPqJsX0kNmVBW+p0v2d5EPNi38+tkFmqDcbeBBK7XLfyBi8hLKs7kncGwTg60DIzZRzsvJf9lMwC8fuiQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8114
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,UPPERCASE_75_100,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 26 Oct 2023 03:56:57 -0700 (PDT)


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,
>>>  };
>>>