Messages in this thread | | | Date | Sun, 10 Dec 2023 11:37:32 +0000 | From | Simon Horman <> | Subject | Re: [PATCH net-next v2 1/4] octeon_ep: add PF-VF mailbox communication |
| |
On Sat, Dec 09, 2023 at 12:14:47AM -0800, Shinas Rasheed wrote: > Implement mailbox communication between PF and VFs. > PF-VF mailbox is used for all control commands from VF to PF and > asynchronous notification messages from PF to VF. > > Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
...
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
...
> @@ -1315,6 +1335,7 @@ static void octep_device_cleanup(struct octep_device *oct) > oct->mbox[i] = NULL; > } > > + octep_delete_pfvf_mbox(oct); > octep_ctrl_net_uninit(oct); > cancel_delayed_work_sync(&oct->hb_task); > > @@ -1411,6 +1432,13 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > goto err_octep_config; > } > > + err = octep_setup_pfvf_mbox(octep_dev); > + if (err) { > + dev_err(&pdev->dev, " pfvf mailbox setup failed\n"); > + octep_ctrl_net_uninit(octep_dev); > + return err;
Hi Shinas,
This seems inconsistent with other error handling in this function, I suspect it is leaking resources. And even if it does not, it's likely to lead to such problems as this function is updated in future. Please consider consistently handling error unwinding using goto labels in this function.
I think that means either making sure that octep_delete_pfvf_mbox() can handle the case whereby octep_setup_pfvf_mbox() fails. Or including) the steps taken by octep_device_cleanup() in the unwind ladder provided by goto labels (which could be a separate patch, and I suspect to be the best approach). But I could well be wrong.
> + } > + > octep_ctrl_net_get_info(octep_dev, OCTEP_CTRL_NET_INVALID_VFID, > &octep_dev->conf->fw_info); > dev_info(&octep_dev->pdev->dev, "Heartbeat interval %u msecs Heartbeat miss count %u\n",
...
| |