On 3/11/24 11:32 AM, Breno Leitao wrote: > Hello Dennis, > > On Mon, Mar 11, 2024 at 08:05:45AM -0400, Dennis Dalessandro wrote: >> On 3/8/24 1:29 PM, Breno Leitao wrote: >>> struct net_device shouldn't be embedded into any structure, instead, >>> the owner should use the priv space to embed their state into net_device. >>> >>> Embedding net_device into structures prohibits the usage of flexible >>> arrays in the net_device structure. For more details, see the discussion >>> at [1]. >>> >>> Un-embed the net_device from struct iwl_trans_pcie by converting it >>> into a pointer. Then use the leverage alloc_netdev() to allocate the >>> net_device object at iwl_trans_pcie_alloc. >> >> What does an Omni-Path Architecture driver from Cornelis Networks have to do >> with an Intel wireless driver? > > That is an oversight. I will fix it in v2. Sorry about it. > >>> The private data of net_device becomes a pointer for the struct >>> iwl_trans_pcie, so, it is easy to get back to the iwl_trans_pcie parent >>> given the net_device object. >>> >>> [1] https://lore.kernel.org/all/20240229225910.79e224cf@xxxxxxxxxx/ >>> >>> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx> >>> --- >>> drivers/infiniband/hw/hfi1/netdev.h | 2 +- >>> drivers/infiniband/hw/hfi1/netdev_rx.c | 9 +++++++-- >>> 2 files changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/infiniband/hw/hfi1/netdev.h b/drivers/infiniband/hw/hfi1/netdev.h >>> index 8aa074670a9c..07c8f77c9181 100644 >>> --- a/drivers/infiniband/hw/hfi1/netdev.h >>> +++ b/drivers/infiniband/hw/hfi1/netdev.h >>> @@ -49,7 +49,7 @@ struct hfi1_netdev_rxq { >>> * When 0 receive queues will be freed. >>> */ >>> struct hfi1_netdev_rx { >>> - struct net_device rx_napi; >>> + struct net_device *rx_napi; >>> struct hfi1_devdata *dd; >>> struct hfi1_netdev_rxq *rxq; >>> int num_rx_q; >>> diff --git a/drivers/infiniband/hw/hfi1/netdev_rx.c b/drivers/infiniband/hw/hfi1/netdev_rx.c >>> index 720d4c85c9c9..5c26a69fa2bb 100644 >>> --- a/drivers/infiniband/hw/hfi1/netdev_rx.c >>> +++ b/drivers/infiniband/hw/hfi1/netdev_rx.c >>> @@ -188,7 +188,7 @@ static int hfi1_netdev_rxq_init(struct hfi1_netdev_rx *rx) >>> int i; >>> int rc; >>> struct hfi1_devdata *dd = rx->dd; >>> - struct net_device *dev = &rx->rx_napi; >>> + struct net_device *dev = rx->rx_napi; >>> >>> rx->num_rx_q = dd->num_netdev_contexts; >>> rx->rxq = kcalloc_node(rx->num_rx_q, sizeof(*rx->rxq), >>> @@ -360,7 +360,11 @@ int hfi1_alloc_rx(struct hfi1_devdata *dd) >>> if (!rx) >>> return -ENOMEM; >>> rx->dd = dd; >>> - init_dummy_netdev(&rx->rx_napi); >>> + rx->rx_napi = alloc_netdev(sizeof(struct iwl_trans_pcie *), >>> + "dummy", NET_NAME_UNKNOWN, >>> + init_dummy_netdev); >> >> Again with the iwl stuff? Please do not stuff to the mailing list that doesn't >> even compile.... >> >> CC [M] drivers/infiniband/hw/hfi1/verbs.o >> CC [M] drivers/infiniband/hw/hfi1/verbs_txreq.o >> CC [M] drivers/infiniband/hw/hfi1/vnic_main.o >> In file included from ./include/net/sock.h:46, >> from ./include/linux/tcp.h:19, >> from ./include/linux/ipv6.h:95, >> from ./include/net/ipv6.h:12, >> from ./include/rdma/ib_verbs.h:25, >> from ./include/rdma/ib_hdrs.h:11, >> from drivers/infiniband/hw/hfi1/hfi.h:29, >> from drivers/infiniband/hw/hfi1/sdma.h:15, >> from drivers/infiniband/hw/hfi1/netdev_rx.c:11: >> drivers/infiniband/hw/hfi1/netdev_rx.c: In function ‘hfi1_alloc_rx’: >> drivers/infiniband/hw/hfi1/netdev_rx.c:365:36: error: passing argument 4 of >> ‘alloc_netdev_mqs’ from incompatible pointer type >> [-Werror=incompatible-pointer-types] >> 365 | init_dummy_netdev); >> | ^~~~~~~~~~~~~~~~~ >> | | >> | int (*)(struct net_device *) >> ./include/linux/netdevice.h:4632:63: note: in definition of macro ‘alloc_netdev’ >> 4632 | alloc_netdev_mqs(sizeof_priv, name, name_assign_type, setup, 1, 1) >> | ^~~~~ >> ./include/linux/netdevice.h:4629:44: note: expected ‘void (*)(struct net_device >> *)’ but argument is of type ‘int (*)(struct net_device *)’ >> 4629 | void (*setup)(struct net_device *), >> | ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> CC [M] drivers/infiniband/hw/hfi1/vnic_sdma.o > > Sorry, this patch is against net-next and you probably tested in Linus' > upstream. > > You need to have d160c66cda0ac8614 ("net: Do not return value from > init_dummy_netdev()"), which is in net-next, and has this important > change that is necessary for this patch: > > -int init_dummy_netdev(struct net_device *dev); > +void init_dummy_netdev(struct net_device *dev); > > If you are OK with a v2, I will fix the topics reported in this thread. You also use struct iwl_trans_pcie in hfi1 code. Fix that up, and make sure the patch builds and I'll give it a test. -Denny > Thank you > Breno
