netdev - Re: [v4,net-next,06/11] net/nebula-matrix: add common resource implementation

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: <20260207022455.4017894-1-kuba@kernel.org>
Date: Fri,  6 Feb 2026 18:24:55 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: illusion.wang@...ula-matrix.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	sam.chen@...ula-matrix.com,
	lukas.bulwahn@...hat.com,
	dimon.zhao@...ula-matrix.com,
	netdev@...r.kernel.org,
	vadim.fedorenko@...ux.dev,
	linux-kernel@...r.kernel.org (open list),
	corbet@....net,
	lorenzo@...nel.org,
	alvin.wang@...ula-matrix.com,
	andrew+netdev@...n.ch,
	linux-doc@...r.kernel.org,
	pabeni@...hat.com,
	edumazet@...gle.com,
	horms@...nel.org
Subject: Re: [v4,net-next,06/11] net/nebula-matrix: add common resource implementation

This is an AI-generated review of your patch. The human sending this
email says "This is an automated AI code reviewer, please treat what
it says with a grain of salt."

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/nebula-matrix: add common resource implementation

This patch adds the common resource layer for the nebula-matrix network
driver, implementing conversion relationships among vsi_id, func_id,
eth_id, and pf_id.

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_resource_leonis.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_resource_leonis.c
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_resource_leonis.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_hw_leonis/nbl_resource_leonis.c

[ ... ]

> +static void nbl_res_get_eth_id(struct nbl_resource_mgt *res_mgt, u16 vsi_id,
> +			       u8 *eth_mode, u8 *eth_id, u8 *logic_eth_id)
> +{
> +	struct nbl_eth_info *eth_info = NBL_RES_MGT_TO_ETH_INFO(res_mgt);
> +	u16 pf_id = nbl_res_vsi_id_to_pf_id(res_mgt, vsi_id);
> +
> +	*eth_mode = eth_info->eth_num;
> +	if (pf_id < eth_info->eth_num) {
> +		*eth_id = eth_info->eth_id[pf_id];
> +		*logic_eth_id = pf_id;
> +		/* if pf_id > eth_num, use eth_id 0 */
> +	} else {

This isn't a bug, but the comment appears to be misplaced. It describes
what happens in the else branch but is located inside the if branch. Should
it be moved above the else?

[ ... ]

> +static int nbl_res_ctrl_dev_setup_eth_info(struct nbl_resource_mgt *res_mgt)
> +{

[ ... ]

> +	for (i = 0; i < NBL_RES_MGT_TO_PF_NUM(res_mgt); i++) {
> +		/* if pf_id <= eth_num, the pf relate corresponding eth_id*/
> +		if (i < eth_num) {

This isn't a bug, but the comment says "pf_id <= eth_num" while the code
checks "i < eth_num". Should the comment say "<" instead of "<="?

> +			eth_id = eth_info->eth_id[i];
> +			eth_info->pf_bitmap[eth_id] |= BIT(i);
> +		}
> +		/* if pf_id > eth_num, the pf relate eth 0*/
> +		else

Similarly, the comment says "pf_id > eth_num" but the else handles the case
where "i >= eth_num". Should the comment say ">=" instead of ">"?

> +			eth_info->pf_bitmap[0] |= BIT(i);
> +	}

[ ... ]

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c

[ ... ]

> +static int vsi_id_to_pf_id(struct nbl_resource_mgt *res_mgt, u16 vsi_id)
> +{
> +	struct nbl_vsi_info *vsi_info = NBL_RES_MGT_TO_VSI_INFO(res_mgt);
> +	struct nbl_common_info *common = res_mgt->common;
> +	bool vsi_find = false;
> +	u32 pf_id = U32_MAX;
> +	int i, j;
> +
> +	for (i = 0; i < vsi_info->num; i++) {
> +		for (j = 0; j < NBL_VSI_SERV_MAX_TYPE; j++)
> +			if (vsi_id >= vsi_info->serv_info[i][j].base_id &&
> +			    (vsi_id < vsi_info->serv_info[i][j].base_id +
> +					      vsi_info->serv_info[i][j].num)) {
> +				vsi_find = true;
> +				break;
> +			}
> +
> +		if (vsi_find)
> +			break;
> +	}
> +
> +	if (vsi_find) {
> +		/* if pf_id < eth_num */
> +		if (j >= NBL_VSI_SERV_PF_DATA_TYPE &&
> +		    j <= NBL_VSI_SERV_PF_DATA_TYPE)

This isn't a bug, but the comment says "if pf_id < eth_num" while the code
checks if j equals NBL_VSI_SERV_PF_DATA_TYPE (which is 0). The comment does
not seem to describe the actual condition being tested. Could it be updated
to reflect what the code is checking?

> +			pf_id = i + common->mgt_pf;
> +	}
> +
> +	return pf_id;
> +}