| 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
| ||
|
Message-ID: <53206f29-7da8-4145-aef0-7bdacef3bb55@linux.dev>
Date: Tue, 26 Aug 2025 18:06:42 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Fan Gong <gongfan1@...wei.com>, Zhu Yikai <zhuyikai1@...artners.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
linux-doc@...r.kernel.org, Jonathan Corbet <corbet@....net>,
Bjorn Helgaas <helgaas@...nel.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>, Meny Yossefi <meny.yossefi@...wei.com>,
Gur Stavi <gur.stavi@...wei.com>, Lee Trager <lee@...ger.us>,
Michael Ellerman <mpe@...erman.id.au>, Suman Ghosh <sumang@...vell.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>
Subject: Re: [PATCH net-next v01 10/12] hinic3: Add Rss function
On 26/08/2025 10:05, Fan Gong wrote:
> Initialize rss functions. Configure rss hash data and HW resources.
>
> Co-developed-by: Xin Guo <guoxin09@...wei.com>
> Signed-off-by: Xin Guo <guoxin09@...wei.com>
> Co-developed-by: Zhu Yikai <zhuyikai1@...artners.com>
> Signed-off-by: Zhu Yikai <zhuyikai1@...artners.com>
> Signed-off-by: Fan Gong <gongfan1@...wei.com>
> ---
> drivers/net/ethernet/huawei/hinic3/Makefile | 1 +
> .../net/ethernet/huawei/hinic3/hinic3_main.c | 9 +-
> .../huawei/hinic3/hinic3_mgmt_interface.h | 55 +++
> .../huawei/hinic3/hinic3_netdev_ops.c | 18 +
> .../ethernet/huawei/hinic3/hinic3_nic_dev.h | 5 +
> .../net/ethernet/huawei/hinic3/hinic3_rss.c | 359 ++++++++++++++++++
> .../net/ethernet/huawei/hinic3/hinic3_rss.h | 14 +
> 7 files changed, 460 insertions(+), 1 deletion(-)
[...]
> +static int alloc_rss_resource(struct net_device *netdev)
> +{
> + struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> + static const u8 default_rss_key[L2NIC_RSS_KEY_SIZE] = {
> + 0x6d, 0x5a, 0x56, 0xda, 0x25, 0x5b, 0x0e, 0xc2,
> + 0x41, 0x67, 0x25, 0x3d, 0x43, 0xa3, 0x8f, 0xb0,
> + 0xd0, 0xca, 0x2b, 0xcb, 0xae, 0x7b, 0x30, 0xb4,
> + 0x77, 0xcb, 0x2d, 0xa3, 0x80, 0x30, 0xf2, 0x0c,
> + 0x6a, 0x42, 0xb7, 0x3b, 0xbe, 0xac, 0x01, 0xfa};
> +
> + nic_dev->rss_hkey = kzalloc(L2NIC_RSS_KEY_SIZE, GFP_KERNEL);
no need to request zero'ed allocation if you are going to overwrite it
completely on the very next line.
> + if (!nic_dev->rss_hkey)
> + return -ENOMEM;
> +
> + memcpy(nic_dev->rss_hkey, default_rss_key, L2NIC_RSS_KEY_SIZE);
I would better move this line after both allocations when the code flow
has no way to fail.
> + nic_dev->rss_indir = kcalloc(L2NIC_RSS_INDIR_SIZE, sizeof(u32),
> + GFP_KERNEL);
why do you allocate L2NIC_RSS_INDIR_SIZE of u32 when the HW table has
le16 type for the entry?
> + if (!nic_dev->rss_indir) {
> + kfree(nic_dev->rss_hkey);
> + nic_dev->rss_hkey = NULL;
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int hinic3_rss_set_indir_tbl(struct hinic3_hwdev *hwdev,
> + const u32 *indir_table)
> +{
> + struct l2nic_cmd_rss_set_indir_tbl *indir_tbl;
> + struct hinic3_cmd_buf *cmd_buf;
> + __le64 out_param;
> + int err;
> + u32 i;
> +
> + cmd_buf = hinic3_alloc_cmd_buf(hwdev);
> + if (!cmd_buf) {
> + dev_err(hwdev->dev, "Failed to allocate cmd buf\n");
> + return -ENOMEM;
> + }
> +
> + cmd_buf->size = cpu_to_le16(sizeof(struct l2nic_cmd_rss_set_indir_tbl));
> + indir_tbl = cmd_buf->buf;
> + memset(indir_tbl, 0, sizeof(*indir_tbl));
> +
> + for (i = 0; i < L2NIC_RSS_INDIR_SIZE; i++)
> + indir_tbl->entry[i] = cpu_to_le16((u16)indir_table[i]);
> +
> + hinic3_cmdq_buf_swab32(indir_tbl, sizeof(*indir_tbl));
> +
> + err = hinic3_cmdq_direct_resp(hwdev, MGMT_MOD_L2NIC,
> + L2NIC_UCODE_CMD_SET_RSS_INDIR_TBL,
> + cmd_buf, &out_param);
> + if (err || out_param != 0) {
no need for "!= 0"
> + dev_err(hwdev->dev, "Failed to set rss indir table\n");
> + err = -EFAULT;
> + }
> +
> + hinic3_free_cmd_buf(hwdev, cmd_buf);
> +
> + return err;
> +}
[...]
> +static int hinic3_rss_cfg_hash_key(struct hinic3_hwdev *hwdev, u8 opcode,
> + u8 *key)
> +{
> + struct l2nic_cmd_cfg_rss_hash_key hash_key = {};
> + struct mgmt_msg_params msg_params = {};
> + int err;
> +
> + hash_key.func_id = hinic3_global_func_id(hwdev);
> + hash_key.opcode = opcode;
> +
> + if (opcode == MGMT_MSG_CMD_OP_SET)
> + memcpy(hash_key.key, key, L2NIC_RSS_KEY_SIZE);
here you copy hash key to a stack allocated structure ...
> +
> + mgmt_msg_params_init_default(&msg_params, &hash_key, sizeof(hash_key));
... which is copied to another stack allocated structure ...
> +
> + err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC,
> + L2NIC_CMD_CFG_RSS_HASH_KEY, &msg_params);
> + if (err || hash_key.msg_head.status) {
> + dev_err(hwdev->dev, "Failed to %s hash key, err: %d, status: 0x%x\n",
> + opcode == MGMT_MSG_CMD_OP_SET ? "set" : "get",
> + err, hash_key.msg_head.status);
> + return -EINVAL;
> + }
> +
> + if (opcode == MGMT_MSG_CMD_OP_GET)
> + memcpy(key, hash_key.key, L2NIC_RSS_KEY_SIZE);
> +
> + return 0;
> +}
> +
> +static int hinic3_rss_set_hash_key(struct hinic3_hwdev *hwdev, const u8 *key)
> +{
> + u8 hash_key[L2NIC_RSS_KEY_SIZE];
> +
> + memcpy(hash_key, key, L2NIC_RSS_KEY_SIZE);
... but it was already copied to stack allocated buffer ...
> +
> + return hinic3_rss_cfg_hash_key(hwdev, MGMT_MSG_CMD_OP_SET, hash_key);
> +}
> +
> +static int hinic3_set_hw_rss_parameters(struct net_device *netdev, u8 rss_en)
> +{
> + struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> + int err;
> +
> + err = hinic3_rss_set_hash_key(nic_dev->hwdev, nic_dev->rss_hkey);
... which is previously copied from static array.
It's 4 copies in total to configure one simple thing. Looks like too
much of copying with no good reason
> + if (err)
> + return err;
> +
Powered by blists - more mailing lists