linux-kernel - Re: [PATCH net 1/3] net: hibmcge: fix rx buf avl irq is not re-enabled in irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc7362e8-e8ae-4813-a73b-d752b403332a@huawei.com>
Date: Fri, 24 Oct 2025 14:39:00 +0800
From: Jijie Shao <shaojijie@...wei.com>
To: Jacob Keller <jacob.e.keller@...el.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<andrew+netdev@...n.ch>, <horms@...nel.org>
CC: <shaojijie@...wei.com>, <shenjian15@...wei.com>, <liuyonglong@...wei.com>,
	<chenhao418@...wei.com>, <lantao5@...wei.com>,
	<huangdonghua3@...artners.com>, <yangshuaisong@...artners.com>,
	<jonathan.cameron@...wei.com>, <salil.mehta@...wei.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 1/3] net: hibmcge: fix rx buf avl irq is not
 re-enabled in irq_handle issue


on 2025/10/24 9:15, Jacob Keller wrote:
>
> On 10/21/2025 7:00 AM, Jijie Shao wrote:
>> irq initialized with the macro HBG_ERR_IRQ_I will automatically
>> be re-enabled, whereas those initialized with the macro HBG_IRQ_I
>> will not be re-enabled.
>>
>> Since the rx buf avl irq is initialized using the macro HBG_IRQ_I,
>> it needs to be actively re-enabled.
>>
> This seems like it would be quite a severe issue. Do you have
> reproduction or example of what the failure state looks like?

priv->stats.rx_fifo_less_empty_thrsld_cnt can only be increased to 1
and cannot be increased further.

It is not a very serious issue, it affects the accuracy of a statistical item.

>
>  From the fixed commit, the RX_BUF_AVL used to be HBG_ERR_IRQ_I but now
> it uses HBG_IRQ_I so that it can have its own custom handler.. but
> HBG_IRQ_I doesn't set re_enable to true...
>
> It seems like a better fix would be having an HBG_ERR_IRQ_I variant that
> lets you pass your own function instead of making the handler have to do
> the hbg_hw_irq_enable call in its handler?

Currently, only the RX_BUF_AVL interrupt needs to be enabled separately.
Personally, I think it is acceptable to temporarily not use the an HBG_ERR_IRQ_I variant.

>
>> Fixes: fd394a334b1c ("net: hibmcge: Add support for abnormal irq handling feature")
>> Signed-off-by: Jijie Shao <shaojijie@...wei.com>
>> ---
>>   drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
>> index 8af0bc4cca21..ae4cb35186d8 100644
>> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
>> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
>> @@ -32,6 +32,7 @@ static void hbg_irq_handle_rx_buf_val(struct hbg_priv *priv,
>>   				      const struct hbg_irq_info *irq_info)
>>   {
>>   	priv->stats.rx_fifo_less_empty_thrsld_cnt++;
>> +	hbg_hw_irq_enable(priv, irq_info->mask, true);
>>   }
>>   
>>   #define HBG_IRQ_I(name, handle) \