lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADEbmW3K7QkfniBtmMt=SZtwZWez30F+sM=656wqmZR8=ig1jQ@mail.gmail.com> Date: Mon, 4 Dec 2023 23:13:24 +0100 From: Michal Schmidt <mschmidt@...hat.com> To: Shinas Rasheed <srasheed@...vell.com> Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, hgani@...vell.com, vimleshk@...vell.com, egallen@...hat.com, pabeni@...hat.com, horms@...nel.org, kuba@...nel.org, davem@...emloft.net, wizhao@...hat.com, konguyen@...hat.com, Veerasenareddy Burru <vburru@...vell.com>, Sathesh Edara <sedara@...vell.com>, Eric Dumazet <edumazet@...gle.com> Subject: Re: [PATCH net v1] octeon_ep: initialise control mbox tasks before using APIs On Mon, Dec 4, 2023 at 11:10 PM Michal Schmidt <mschmidt@...hat.com> wrote: > > On Sat, Dec 2, 2023 at 4:08 PM Shinas Rasheed <srasheed@...vell.com> wrote: > > Do INIT_WORK for the various workqueue tasks before the first > > invocation of any control net APIs. Since octep_ctrl_net_get_info > > was called before the control net receive work task was even > > initialised, the function call wasn't returning actual firmware > > info queried from Octeon. > > It might be more accurate to say that octep_ctrl_net_get_info depends > on the processing of OEI events. This happens in intr_poll_task. > That's why intr_poll_task needs to be queued earlier. > Did octep_send_mbox_req previously always fail with EAGAIN after ^^^^^^^^^^^^^^^^^^^^^ I meant octep_ctrl_net_get_info here. > running into the 500 ms timeout in octep_send_mbox_req? > > Apropos octep_send_mbox_req... I think it has a race. "d" is put on > the ctrl_req_wait_list after sending the request to the hardware. If > the response arrives quickly, "d" might not yet be on the list when > process_mbox_resp looks for it. > Also, what protects ctrl_req_wait_list from concurrent access? > > Michal > > > Fixes: 8d6198a14e2b ("octeon_ep: support to fetch firmware info") > > Signed-off-by: Shinas Rasheed <srasheed@...vell.com> > > --- > > .../net/ethernet/marvell/octeon_ep/octep_main.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c > > index 552970c7dec0..3e7bfd3e0f56 100644 > > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c > > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c > > @@ -1193,6 +1193,13 @@ int octep_device_setup(struct octep_device *oct) > > if (ret) > > return ret; > > > > + INIT_WORK(&oct->tx_timeout_task, octep_tx_timeout_task); > > + INIT_WORK(&oct->ctrl_mbox_task, octep_ctrl_mbox_task); > > + INIT_DELAYED_WORK(&oct->intr_poll_task, octep_intr_poll_task); > > + oct->poll_non_ioq_intr = true; > > + queue_delayed_work(octep_wq, &oct->intr_poll_task, > > + msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS)); > > + > > atomic_set(&oct->hb_miss_cnt, 0); > > INIT_DELAYED_WORK(&oct->hb_task, octep_hb_timeout_task); > > > > @@ -1333,13 +1340,6 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > queue_delayed_work(octep_wq, &octep_dev->hb_task, > > msecs_to_jiffies(octep_dev->conf->fw_info.hb_interval)); > > > > - INIT_WORK(&octep_dev->tx_timeout_task, octep_tx_timeout_task); > > - INIT_WORK(&octep_dev->ctrl_mbox_task, octep_ctrl_mbox_task); > > - INIT_DELAYED_WORK(&octep_dev->intr_poll_task, octep_intr_poll_task); > > - octep_dev->poll_non_ioq_intr = true; > > - queue_delayed_work(octep_wq, &octep_dev->intr_poll_task, > > - msecs_to_jiffies(OCTEP_INTR_POLL_TIME_MSECS)); > > - > > netdev->netdev_ops = &octep_netdev_ops; > > octep_set_ethtool_ops(netdev); > > netif_carrier_off(netdev); > > -- > > 2.25.1 > >
Powered by blists - more mailing lists