[PATCH net 0/4] fix some issues in Huawei hinic driver

[PATCH net 0/4] fix some issues in Huawei hinic driver

[Zhengchao Shao]

Fix some issues in Huawei hinic driver. This patchset is compiled only, 
not tested.

Zhengchao Shao (4):
  net: hinic: fix incorrect assignment issue in
    hinic_set_interrupt_cfg()
  net: hinic: fix memory leak when reading function table
  net: hinic: fix the issue of CMDQ memory leaks
  net: hinic: fix the issue of double release MBOX callback of VF

 .../net/ethernet/huawei/hinic/hinic_debugfs.c  | 18 ++++++++++++------
 .../net/ethernet/huawei/hinic/hinic_hw_cmdq.c  |  5 +++++
 .../net/ethernet/huawei/hinic/hinic_hw_dev.c   |  2 +-
 .../net/ethernet/huawei/hinic/hinic_sriov.c    |  1 -
 4 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH net 1/4] net: hinic: fix incorrect assignment issue in hinic_set_interrupt_cfg()

[Zhengchao Shao]

The value of lli_credit_cnt is incorrectly assigned, fix it.

Fixes: a0337c0dee68 ("hinic: add support to set and get irq coalesce")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
index 94f470556295..27795288c586 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
@@ -877,7 +877,7 @@ int hinic_set_interrupt_cfg(struct hinic_hwdev *hwdev,
 	if (err)
 		return -EINVAL;
 
-	interrupt_info->lli_credit_cnt = temp_info.lli_timer_cnt;
+	interrupt_info->lli_credit_cnt = temp_info.lli_credit_cnt;
 	interrupt_info->lli_timer_cnt = temp_info.lli_timer_cnt;
 
 	err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM,
-- 
2.17.1

[PATCH net 2/4] net: hinic: fix memory leak when reading function table

[Zhengchao Shao]

When the input parameter idx meets the expected case option in
hinic_dbg_get_func_table(), read_data is not released. Fix it.

Fixes: 5215e16244ee ("hinic: add support to query function table")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_debugfs.c  | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c
index 19eb839177ec..061952c6c21a 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c
@@ -85,6 +85,7 @@ static int hinic_dbg_get_func_table(struct hinic_dev *nic_dev, int idx)
 	struct tag_sml_funcfg_tbl *funcfg_table_elem;
 	struct hinic_cmd_lt_rd *read_data;
 	u16 out_size = sizeof(*read_data);
+	int ret = ~0;
 	int err;
 
 	read_data = kzalloc(sizeof(*read_data), GFP_KERNEL);
@@ -111,20 +112,25 @@ static int hinic_dbg_get_func_table(struct hinic_dev *nic_dev, int idx)
 
 	switch (idx) {
 	case VALID:
-		return funcfg_table_elem->dw0.bs.valid;
+		ret = funcfg_table_elem->dw0.bs.valid;
+		break;
 	case RX_MODE:
-		return funcfg_table_elem->dw0.bs.nic_rx_mode;
+		ret = funcfg_table_elem->dw0.bs.nic_rx_mode;
+		break;
 	case MTU:
-		return funcfg_table_elem->dw1.bs.mtu;
+		ret = funcfg_table_elem->dw1.bs.mtu;
+		break;
 	case RQ_DEPTH:
-		return funcfg_table_elem->dw13.bs.cfg_rq_depth;
+		ret = funcfg_table_elem->dw13.bs.cfg_rq_depth;
+		break;
 	case QUEUE_NUM:
-		return funcfg_table_elem->dw13.bs.cfg_q_num;
+		ret = funcfg_table_elem->dw13.bs.cfg_q_num;
+		break;
 	}
 
 	kfree(read_data);
 
-	return ~0;
+	return ret;
 }
 
 static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t count,
-- 
2.17.1

[PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[Zhengchao Shao]

When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
not released correctly. Fix it.

Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
index 78190e88cd75..2a759b9bb6b6 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
@@ -877,6 +877,7 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif,
 {
 	struct hinic_func_to_io *func_to_io = cmdqs_to_func_to_io(cmdqs);
 	struct pci_dev *pdev = hwif->pdev;
+	enum hinic_cmdq_type cmdq_type;
 	struct hinic_hwdev *hwdev;
 	u16 max_wqe_size;
 	int err;
@@ -925,6 +926,10 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif,
 err_set_cmdq_depth:
 	hinic_ceq_unregister_cb(&func_to_io->ceqs, HINIC_CEQ_CMDQ);
 
+	cmdq_type = HINIC_CMDQ_SYNC;
+	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)
+		free_cmdq(&cmdqs->cmdq[cmdq_type]);
+
 err_cmdq_ctxt:
 	hinic_wqs_cmdq_free(&cmdqs->cmdq_pages, cmdqs->saved_wqs,
 			    HINIC_MAX_CMDQ_TYPES);
-- 
2.17.1

[PATCH net 4/4] net: hinic: fix the issue of double release MBOX callback of VF

[Zhengchao Shao]

In hinic_vf_func_init(), if VF fails to register information with PF
through the MBOX, the MBOX callback function of VF is released once. But
it is released again in hinic_init_hwdev(). Remove one.

Fixes: 7dd29ee12865 ("hinic: add sriov feature support")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/ethernet/huawei/hinic/hinic_sriov.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
index a5f08b969e3f..f7e05b41385b 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
@@ -1174,7 +1174,6 @@ int hinic_vf_func_init(struct hinic_hwdev *hwdev)
 			dev_err(&hwdev->hwif->pdev->dev,
 				"Failed to register VF, err: %d, status: 0x%x, out size: 0x%x\n",
 				err, register_info.status, out_size);
-			hinic_unregister_vf_mbox_cb(hwdev, HINIC_MOD_L2NIC);
 			return -EIO;
 		}
 	} else {
-- 
2.17.1

Re: [PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[Leon Romanovsky]

On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote:
> When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
> not released correctly. Fix it.
> 
> Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> ---
>  drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
>  1 file changed, 5 insertions(+)

<...>

> +	cmdq_type = HINIC_CMDQ_SYNC;
> +	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)

Why do you have this "for loops" in all places? There is only one cmdq_type.

Thanks

Re: [PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[shaozhengchao]

On 2022/10/19 15:20, Leon Romanovsky wrote:
> On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote:
>> When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
>> not released correctly. Fix it.
>>
>> Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>> ---
>>   drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
>>   1 file changed, 5 insertions(+)
> 
> <...>
> 
>> +	cmdq_type = HINIC_CMDQ_SYNC;
>> +	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)
> 
> Why do you have this "for loops" in all places? There is only one cmdq_type.
> 
> Thanks
Hi Leon:
	Thank you for your review. Now, only the synchronous CMDQ is
enabled for the current CMDQs. New type of CMDQ could be added later.
So looping style is maintained on both the allocation and release paths.

Zhengchao Shao

Re: [PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[Leon Romanovsky]

On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote:
> 
> 
> On 2022/10/19 15:20, Leon Romanovsky wrote:
> > On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote:
> > > When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
> > > not released correctly. Fix it.
> > > 
> > > Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
> > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> > > ---
> > >   drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > 
> > <...>
> > 
> > > +	cmdq_type = HINIC_CMDQ_SYNC;
> > > +	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)
> > 
> > Why do you have this "for loops" in all places? There is only one cmdq_type.
> > 
> > Thanks
> Hi Leon:
> 	Thank you for your review. Now, only the synchronous CMDQ is
> enabled for the current CMDQs. New type of CMDQ could be added later.

Single command type was added in 2017, and five years later, new type wasn't added yet.

> So looping style is maintained on both the allocation and release paths.
> 
> Zhengchao Shao

Re: [PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[shaozhengchao]

On 2022/10/19 16:39, Leon Romanovsky wrote:
> On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote:
>>
>>
>> On 2022/10/19 15:20, Leon Romanovsky wrote:
>>> On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote:
>>>> When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
>>>> not released correctly. Fix it.
>>>>
>>>> Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
>>>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>>>> ---
>>>>    drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>
>>> <...>
>>>
>>>> +	cmdq_type = HINIC_CMDQ_SYNC;
>>>> +	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)
>>>
>>> Why do you have this "for loops" in all places? There is only one cmdq_type.
>>>
>>> Thanks
>> Hi Leon:
>> 	Thank you for your review. Now, only the synchronous CMDQ is
>> enabled for the current CMDQs. New type of CMDQ could be added later.
> 
> Single command type was added in 2017, and five years later, new type wasn't added yet.
> 
OK, I will modify in V2, and I will do cleanup in another patch.

Thanks

Zhengchao Shao

>> So looping style is maintained on both the allocation and release paths.
>>
>> Zhengchao Shao

Re: [PATCH net 3/4] net: hinic: fix the issue of CMDQ memory leaks

[Leon Romanovsky]

On Wed, Oct 19, 2022 at 05:37:42PM +0800, shaozhengchao wrote:
> 
> 
> On 2022/10/19 16:39, Leon Romanovsky wrote:
> > On Wed, Oct 19, 2022 at 03:41:06PM +0800, shaozhengchao wrote:
> > > 
> > > 
> > > On 2022/10/19 15:20, Leon Romanovsky wrote:
> > > > On Wed, Oct 19, 2022 at 10:42:19AM +0800, Zhengchao Shao wrote:
> > > > > When hinic_set_cmdq_depth() fails in hinic_init_cmdqs(), the cmdq memory is
> > > > > not released correctly. Fix it.
> > > > > 
> > > > > Fixes: 72ef908bb3ff ("hinic: add three net_device_ops of vf")
> > > > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> > > > > ---
> > > > >    drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 5 +++++
> > > > >    1 file changed, 5 insertions(+)
> > > > 
> > > > <...>
> > > > 
> > > > > +	cmdq_type = HINIC_CMDQ_SYNC;
> > > > > +	for (; cmdq_type < HINIC_MAX_CMDQ_TYPES; cmdq_type++)
> > > > 
> > > > Why do you have this "for loops" in all places? There is only one cmdq_type.
> > > > 
> > > > Thanks
> > > Hi Leon:
> > > 	Thank you for your review. Now, only the synchronous CMDQ is
> > > enabled for the current CMDQs. New type of CMDQ could be added later.
> > 
> > Single command type was added in 2017, and five years later, new type wasn't added yet.
> > 
> OK, I will modify in V2, and I will do cleanup in another patch.

Thanks

> 
> Thanks
> 
> Zhengchao Shao
> 
> > > So looping style is maintained on both the allocation and release paths.
> > > 
> > > Zhengchao Shao