> Subject: [EXTERNAL] Re: [v2,net-next,1/6] net: mana: Create separate EQs for > each vPort > > This is an AI-generated review of your patch. The human sending this email has > considered the AI review valid, or at least plausible. > > For local reproduction steps see > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnetdev- > ai.bots.linux.dev%2Fai- > local.html&data=05%7C02%7Clongli%40microsoft.com%7C070979f149574e63 > e15a08de7b9aa19e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6 > 39084101208945710%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnR > ydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3 > D%3D%7C0%7C%7C%7C&sdata=Cs%2BZwNguYa8FGbpb4H0naVDhDzhjzBvDU7 > 5aZekerzc%3D&reserved=0 > --- > net: mana: Create separate EQs for each vPort > > To prepare for assigning vPorts to dedicated MSI-X vectors, this commit > removes EQ sharing among vPorts and creates dedicated EQs for each vPort. > The EQ definition is moved from struct mana_context to struct > mana_port_context and related support functions are updated. > > > diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h > > --- a/include/net/mana/mana.h > > +++ b/include/net/mana/mana.h > > [ ... ] > > > @@ -499,6 +497,9 @@ struct mana_port_context { > > > > u8 mac_addr[ETH_ALEN]; > > > > + struct mana_eq *eqs; > > + struct dentry *mana_eqs_debugfs; > > + > > enum TRI_STATE rss_state; > > > > mana_handle_t default_rxobj; > > [ ... ] > > > diff --git a/drivers/infiniband/hw/mana/qp.c > > b/drivers/infiniband/hw/mana/qp.c > > --- a/drivers/infiniband/hw/mana/qp.c > > +++ b/drivers/infiniband/hw/mana/qp.c > > @@ -188,7 +188,7 @@ struct mana_obj_spec cq_spec = {}; > > cq_spec.gdma_region = cq->queue.gdma_region; > > cq_spec.queue_size = cq->cqe * COMP_ENTRY_SIZE; > > cq_spec.modr_ctx_id = 0; > > - eq = &mpc->ac->eqs[cq->comp_vector]; > > + eq = &mpc->eqs[cq->comp_vector % mpc->num_queues]; > ^^^^^^^ > > cq_spec.attached_eq = eq->eq->id; > > Can mpc->eqs be NULL here? Looking at the initialization paths: > > Before this patch, mpc->ac->eqs was allocated during mana_probe() and was > always non-NULL after driver initialization. > > After this patch, mpc->eqs is only allocated in two places: > 1. mana_alloc_queues() - called when the Ethernet interface is brought up 2. > mana_ib_cfg_vport() - called when RDMA RAW QPs configure the vport > > However, mana_ib_create_qp_rss() doesn't call mana_ib_cfg_vport(). If an RSS > QP is created via RDMA verbs before the Ethernet port is brought up and > before any RAW QP has configured the vport, mpc->eqs would still be NULL. > > Computing &mpc->eqs[cq->comp_vector % mpc->num_queues] from a NULL > pointer produces an invalid address, and the subsequent dereference eq->eq- > >id would crash the kernel. > > Should there be a NULL check here, or should mana_ib_create_qp_rss() ensure > EQs are created before reaching this point?
I'm fixing it in v3. Thank you, Long

