netdev - Re: [PATCH net-next v04 3/5] hinic3: Add NIC configuration ops

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: <aQsVXfFlZzIeSf-V@horms.kernel.org>
Date: Wed, 5 Nov 2025 09:14:05 +0000
From: Simon Horman <horms@...nel.org>
To: Fan Gong <gongfan1@...wei.com>
Cc: Zhu Yikai <zhuyikai1@...artners.com>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>, Markus.Elfring@....de,
	pavan.chebbi@...adcom.com, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, luosifu <luosifu@...wei.com>,
	Xin Guo <guoxin09@...wei.com>,
	Shen Chenyang <shenchenyang1@...ilicon.com>,
	Zhou Shuai <zhoushuai28@...wei.com>, Wu Like <wulike1@...wei.com>,
	Shi Jing <shijing34@...wei.com>,
	Luo Yang <luoyang82@...artners.com>,
	Meny Yossefi <meny.yossefi@...wei.com>,
	Gur Stavi <gur.stavi@...wei.com>
Subject: Re: [PATCH net-next v04 3/5] hinic3: Add NIC configuration ops

On Wed, Oct 29, 2025 at 02:16:27PM +0800, Fan Gong wrote:

...

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c
> index 09dae2ef610c..0efb5a843964 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c
> @@ -9,6 +9,36 @@
>  #include "hinic3_hwif.h"
>  #include "hinic3_mbox.h"
>  
> +static int hinic3_get_interrupt_cfg(struct hinic3_hwdev *hwdev,
> +				    struct hinic3_interrupt_info *info)
> +{
> +	struct comm_cmd_cfg_msix_ctrl_reg msix_cfg = {};
> +	struct mgmt_msg_params msg_params = {};
> +	int err;
> +
> +	msix_cfg.func_id = hinic3_global_func_id(hwdev);
> +	msix_cfg.msix_index = info->msix_index;
> +	msix_cfg.opcode = MGMT_MSG_CMD_OP_GET;
> +
> +	mgmt_msg_params_init_default(&msg_params, &msix_cfg, sizeof(msix_cfg));
> +
> +	err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_COMM,
> +				       COMM_CMD_CFG_MSIX_CTRL_REG, &msg_params);
> +	if (err || msix_cfg.head.status) {
> +		dev_err(hwdev->dev, "Failed to get interrupt config, err: %d, status: 0x%x\n",
> +			err, msix_cfg.head.status);
> +		return -EFAULT;
> +	}
> +
> +	info->lli_credit_limit = msix_cfg.lli_credit_cnt;
> +	info->lli_timer_cfg = msix_cfg.lli_timer_cnt;
> +	info->pending_limit = msix_cfg.pending_cnt;
> +	info->coalesc_timer_cfg = msix_cfg.coalesce_timer_cnt;
> +	info->resend_timer_cfg = msix_cfg.resend_timer_cnt;
> +
> +	return 0;
> +}
> +
>  int hinic3_set_interrupt_cfg_direct(struct hinic3_hwdev *hwdev,
>  				    const struct hinic3_interrupt_info *info)
>  {
> @@ -40,6 +70,30 @@ int hinic3_set_interrupt_cfg_direct(struct hinic3_hwdev *hwdev,
>  	return 0;
>  }
>  
> +int hinic3_set_interrupt_cfg(struct hinic3_hwdev *hwdev,
> +			     struct hinic3_interrupt_info info)
> +{
> +	struct hinic3_interrupt_info temp_info;
> +	int err;
> +
> +	temp_info.msix_index = info.msix_index;
> +
> +	err = hinic3_get_interrupt_cfg(hwdev, &temp_info);
> +	if (err)
> +		return -EINVAL;

Maybe I am missing something. It seems to me thaat this error value will
propagate up to be the return value of the probe value. And it seems to me
that would be a bit more intuitive, and possibly lead to a better user
experience, if the return value of hinic3_get_interrupt_cfg() was
propagated here. And, in turn, if hinic3_get_interrupt_cfg() propagated the
return value of hinic3_send_mbox_to_mgmt(). These values differ from
-EINVAL.

> +
> +	info.lli_credit_limit = temp_info.lli_credit_limit;
> +	info.lli_timer_cfg = temp_info.lli_timer_cfg;
> +
> +	if (!info.interrupt_coalesc_set) {
> +		info.pending_limit = temp_info.pending_limit;
> +		info.coalesc_timer_cfg = temp_info.coalesc_timer_cfg;
> +		info.resend_timer_cfg = temp_info.resend_timer_cfg;
> +	}
> +
> +	return hinic3_set_interrupt_cfg_direct(hwdev, &info);
> +}

...