[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0b5955cc-f552-2264-2a59-971604b7f7a4@huawei.com>
Date: Sat, 1 Aug 2020 10:27:50 +0800
From: "luobin (L)" <luobin9@...wei.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <davem@...emloft.net>, <linux-kernel@...r.kernel.org>,
<netdev@...r.kernel.org>, <luoxianjun@...wei.com>,
<yin.yinshi@...wei.com>, <cloud.wangxiaoyun@...wei.com>,
<chiqijun@...wei.com>
Subject: Re: [PATCH net-next v2 1/2] hinic: add generating mailbox random
index support
On 2020/8/1 3:52, Jakub Kicinski wrote:
> On Fri, 31 Jul 2020 09:56:41 +0800 Luo bin wrote:
>> add support to generate mailbox random id of VF to ensure that
>> mailbox messages PF received are from the correct VF.
>>
>> Signed-off-by: Luo bin <luobin9@...wei.com>
>
>> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c
>> index 47c93f946b94..c72aa8e8bce8 100644
>> --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c
>> +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c
>> @@ -486,6 +486,111 @@ static void recv_mbox_handler(struct hinic_mbox_func_to_func *func_to_func,
>> kfree(rcv_mbox_temp);
>> }
>>
>> +static int set_vf_mbox_random_id(struct hinic_hwdev *hwdev, u16 func_id)
>> +{
>> + struct hinic_mbox_func_to_func *func_to_func = hwdev->func_to_func;
>> + struct hinic_set_random_id rand_info = {0};
>> + u16 out_size = sizeof(rand_info);
>> + struct hinic_pfhwdev *pfhwdev;
>> + int ret;
>> +
>> + pfhwdev = container_of(hwdev, struct hinic_pfhwdev, hwdev);
>> +
>> + rand_info.version = HINIC_CMD_VER_FUNC_ID;
>> + rand_info.func_idx = func_id;
>> + rand_info.vf_in_pf = (u8)(func_id - hinic_glb_pf_vf_offset(hwdev->hwif));
>
> this cast is unnecessary
>
Will fix. Thanks for your review.
>> + get_random_bytes(&rand_info.random_id, sizeof(u32));
>
> get_random_u32()
>
Will fix. Thanks for your review.
>> +
>> + func_to_func->vf_mbx_rand_id[func_id] = rand_info.random_id;
>> +
>> + ret = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM,
>> + HINIC_MGMT_CMD_SET_VF_RANDOM_ID,
>> + &rand_info, sizeof(rand_info),
>> + &rand_info, &out_size, HINIC_MGMT_MSG_SYNC);
>> + if ((rand_info.status != HINIC_MGMT_CMD_UNSUPPORTED &&
>> + rand_info.status) || !out_size || ret) {
>> + dev_err(&hwdev->hwif->pdev->dev, "Set VF random id failed, err: %d, status: 0x%x, out size: 0x%x\n",
>> + ret, rand_info.status, out_size);
>> + return -EIO;
>> + }
>> +
>> + if (rand_info.status == HINIC_MGMT_CMD_UNSUPPORTED)
>> + return rand_info.status;
>> +
>> + func_to_func->vf_mbx_old_rand_id[func_id] =
>> + func_to_func->vf_mbx_rand_id[func_id];
>> +
>> + return 0;
>> +}
>
>> +static bool check_vf_mbox_random_id(struct hinic_mbox_func_to_func *func_to_func,
>> + u8 *header)
>> +{
>> + struct hinic_hwdev *hwdev = func_to_func->hwdev;
>> + struct hinic_mbox_work *mbox_work = NULL;
>> + u64 mbox_header = *((u64 *)header);
>> + u16 offset, src;
>> + u32 random_id;
>> + int vf_in_pf;
>> +
>> + src = HINIC_MBOX_HEADER_GET(mbox_header, SRC_GLB_FUNC_IDX);
>> +
>> + if (IS_PF_OR_PPF_SRC(src) || !func_to_func->support_vf_random)
>> + return true;
>> +
>> + if (!HINIC_IS_PPF(hwdev->hwif)) {
>> + offset = hinic_glb_pf_vf_offset(hwdev->hwif);
>> + vf_in_pf = src - offset;
>> +
>> + if (vf_in_pf < 1 || vf_in_pf > hwdev->nic_cap.max_vf) {
>> + dev_warn(&hwdev->hwif->pdev->dev,
>> + "Receive vf id(0x%x) is invalid, vf id should be from 0x%x to 0x%x\n",
>> + src, offset + 1,
>> + hwdev->nic_cap.max_vf + offset);
>> + return false;
>> + }
>> + }
>> +
>> + random_id = be32_to_cpu(*(u32 *)(header + MBOX_SEG_LEN +
>> + MBOX_HEADER_SZ));
>> +
>> + if (random_id == func_to_func->vf_mbx_rand_id[src] ||
>> + random_id == func_to_func->vf_mbx_old_rand_id[src])
>
> What guarantees src < MAX_FUNCTION_NUM ?
>
It has been checked if src >= MAX_FUNCTION_NUM in hinic_mbox_func_aeqe_handler before calling this function.
>> + return true;
>> +
>> + dev_warn(&hwdev->hwif->pdev->dev,
>> + "The mailbox random id(0x%x) of func_id(0x%x) doesn't match with pf reservation(0x%x)\n",
>> + random_id, src, func_to_func->vf_mbx_rand_id[src]);
>> +
>> + mbox_work = kzalloc(sizeof(*mbox_work), GFP_KERNEL);
>> + if (!mbox_work)
>> + return false;
>> +
>> + mbox_work->func_to_func = func_to_func;
>> + mbox_work->src_func_idx = src;
>> +
>> + INIT_WORK(&mbox_work->work, update_random_id_work_handler);
>> + queue_work(func_to_func->workq, &mbox_work->work);
>> +
>> + return false;
>> +}
>
>> +int hinic_vf_mbox_random_id_init(struct hinic_hwdev *hwdev)
>> +{
>> + u8 vf_in_pf;
>> + int err = 0;
>> +
>> + if (HINIC_IS_VF(hwdev->hwif))
>> + return 0;
>> +
>> + for (vf_in_pf = 1; vf_in_pf <= hwdev->nic_cap.max_vf; vf_in_pf++) {
>> + err = set_vf_mbox_random_id(hwdev, hinic_glb_pf_vf_offset
>> + (hwdev->hwif) + vf_in_pf);
>
> Parenthesis around hwdev->hwif not necessary
hwdev->hwif is the parameter of hinic_glb_pf_vf_offset function.
>
>> + if (err)
>> + break;
>> + }
>> +
>> + if (err == HINIC_MGMT_CMD_UNSUPPORTED) {
>> + hwdev->func_to_func->support_vf_random = false;
>
> So all VFs need to support the feature for it to be used?
If this feature is not supported by fw, VFs can also be used, so we return success.
>
>> + err = 0;
>> + dev_warn(&hwdev->hwif->pdev->dev, "Mgmt is unsupported to set VF%d random id\n",
>> + vf_in_pf - 1);
>> + } else if (!err) {
>> + hwdev->func_to_func->support_vf_random = true;
>> + dev_info(&hwdev->hwif->pdev->dev, "PF Set VF random id success\n");
>
> Is this info message really necessary?
I'll remove this info message. Thanks.
>
>> + }
>
> .
>
Powered by blists - more mailing lists