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@xxxxxxxxxx>
Signed-off-by: Xin Guo <guoxin09@xxxxxxxxxx>
Co-developed-by: Zhu Yikai <zhuyikai1@xxxxxxxxxxxxxx>
Signed-off-by: Zhu Yikai <zhuyikai1@xxxxxxxxxxxxxx>
Signed-off-by: Fan Gong <gongfan1@xxxxxxxxxx>
---
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;
+
