| Message ID | 20221110013116.270258-1-shaozhengchao@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: liquidio: release resources when liquidio driver open failed | expand |
On Thu, Nov 10, 2022 at 09:31:16AM +0800, Zhengchao Shao wrote: > When liquidio driver open failed, it doesn't release resources. Compile > tested only. > > Fixes: 5b07aee11227 ("liquidio: MSIX support for CN23XX") > Fixes: dbc97bfd3918 ("net: liquidio: Add missing null pointer checks") > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > --- > .../net/ethernet/cavium/liquidio/lio_main.c | 40 ++++++++++++++++--- > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c > index d312bd594935..713689cf212c 100644 > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c > @@ -1795,12 +1795,15 @@ static int liquidio_open(struct net_device *netdev) > ifstate_set(lio, LIO_IFSTATE_RUNNING); > > if (OCTEON_CN23XX_PF(oct)) { > - if (!oct->msix_on) > - if (setup_tx_poll_fn(netdev)) > - return -1; > + if (!oct->msix_on) { > + ret = setup_tx_poll_fn(netdev); > + if (ret) > + goto err_poll; > + } > } else { > - if (setup_tx_poll_fn(netdev)) > - return -1; > + ret = setup_tx_poll_fn(netdev); > + if (ret) > + goto err_poll; > } Instead of this hairy code, you can squeeze everything into one if: if (!OCTEON_CN23XX_PF(oct) || (OCTEON_CN23XX_PF(oct) && oct->msix_on)) { ret = setup_tx_poll_fn(netdev); if (ret) ,,, Thanks
On 2022/11/10 17:45, Leon Romanovsky wrote: > On Thu, Nov 10, 2022 at 09:31:16AM +0800, Zhengchao Shao wrote: >> When liquidio driver open failed, it doesn't release resources. Compile >> tested only. >> >> Fixes: 5b07aee11227 ("liquidio: MSIX support for CN23XX") >> Fixes: dbc97bfd3918 ("net: liquidio: Add missing null pointer checks") >> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> >> --- >> .../net/ethernet/cavium/liquidio/lio_main.c | 40 ++++++++++++++++--- >> 1 file changed, 34 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c >> index d312bd594935..713689cf212c 100644 >> --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c >> +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c >> @@ -1795,12 +1795,15 @@ static int liquidio_open(struct net_device *netdev) >> ifstate_set(lio, LIO_IFSTATE_RUNNING); >> >> if (OCTEON_CN23XX_PF(oct)) { >> - if (!oct->msix_on) >> - if (setup_tx_poll_fn(netdev)) >> - return -1; >> + if (!oct->msix_on) { >> + ret = setup_tx_poll_fn(netdev); >> + if (ret) >> + goto err_poll; >> + } >> } else { >> - if (setup_tx_poll_fn(netdev)) >> - return -1; >> + ret = setup_tx_poll_fn(netdev); >> + if (ret) >> + goto err_poll; >> } > > Instead of this hairy code, you can squeeze everything into one if: > > if (!OCTEON_CN23XX_PF(oct) || (OCTEON_CN23XX_PF(oct) && oct->msix_on)) { > ret = setup_tx_poll_fn(netdev); > if (ret) > ,,, > > Thanks Hi Leon： Thank you for your suggestion. I will modify it in V2. Zhengchao Shao
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index d312bd594935..713689cf212c 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -1795,12 +1795,15 @@ static int liquidio_open(struct net_device *netdev) ifstate_set(lio, LIO_IFSTATE_RUNNING); if (OCTEON_CN23XX_PF(oct)) { - if (!oct->msix_on) - if (setup_tx_poll_fn(netdev)) - return -1; + if (!oct->msix_on) { + ret = setup_tx_poll_fn(netdev); + if (ret) + goto err_poll; + } } else { - if (setup_tx_poll_fn(netdev)) - return -1; + ret = setup_tx_poll_fn(netdev); + if (ret) + goto err_poll; } netif_tx_start_all_queues(netdev); @@ -1813,7 +1816,7 @@ static int liquidio_open(struct net_device *netdev) /* tell Octeon to start forwarding packets to host */ ret = send_rx_ctrl_cmd(lio, 1); if (ret) - return ret; + goto err_rx_ctrl; /* start periodical statistics fetch */ INIT_DELAYED_WORK(&lio->stats_wk.work, lio_fetch_stats); @@ -1824,6 +1827,31 @@ static int liquidio_open(struct net_device *netdev) dev_info(&oct->pci_dev->dev, "%s interface is opened\n", netdev->name); + return 0; + +err_rx_ctrl: + if (OCTEON_CN23XX_PF(oct)) { + if (!oct->msix_on) + cleanup_tx_poll_fn(netdev); + } else { + cleanup_tx_poll_fn(netdev); + } +err_poll: + if (lio->ptp_clock) { + ptp_clock_unregister(lio->ptp_clock); + lio->ptp_clock = NULL; + } + + if (oct->props[lio->ifidx].napi_enabled == 1) { + list_for_each_entry_safe(napi, n, &netdev->napi_list, dev_list) + napi_disable(napi); + + oct->props[lio->ifidx].napi_enabled = 0; + + if (OCTEON_CN23XX_PF(oct)) + oct->droq[0]->ops.poll_mode = 0; + } + return ret; }
When liquidio driver open failed, it doesn't release resources. Compile tested only. Fixes: 5b07aee11227 ("liquidio: MSIX support for CN23XX") Fixes: dbc97bfd3918 ("net: liquidio: Add missing null pointer checks") Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> --- .../net/ethernet/cavium/liquidio/lio_main.c | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-)