> -----Original Message----- > From: Jason Gunthorpe <jgg@xxxxxxxx> > Sent: Tuesday, August 24, 2021 10:42 PM > To: Ariel Elior <aelior@xxxxxxxxxxx> > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Shai Malin > <smalin@xxxxxxxxxxx>; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; > netdev@xxxxxxxxxxxxxxx; malin1024@xxxxxxxxx; RDMA mailing list <linux- > rdma@xxxxxxxxxxxxxxx> > Subject: Re: [EXT] Re: [PATCH] qed: Enable RDMA relaxed ordering > > On Tue, Aug 24, 2021 at 07:16:41PM +0000, Ariel Elior wrote: > > > In our view the qed/qede/qedr/qedi/qedf are separate drivers, hence we > > used function pointer structures for the communication between them. > > We use hierarchies of structures of function pointers to group > > toghether those which have common purposes (dcbx, ll2, Ethernet, > > rdma). Changing that to flat exported functions for the RDMA protocol is no > problem if it is preferred by you. > > I wouldn't twist the driver into knots, but you definately should not be using > function pointers when there is only one implementation, eliminating that > would be a fine start and looks straightforward. > > Many of the functions in the rdma ops do not look complicated to move, yes, > it moves around the layering a bit, but that is OK and probably more > maintainable in the end. eg modify_qp seems fairly disconnected at the first > couple layers of function calls. > > > In summary - we got the message and will work on it, but this is no > > small task and may take some time, and will likely not result in total > > removal of any mention whatsoever of rdma from the core module (but > > will reduce it considerably). > > I wouldn't go for complete removal, you just need to have a core driver with > an exported API that makes some sense for the device. > > eg looking at a random op > > qed_iwarp_set_engine_affin() > > Is an "rdma" function but all it does is call > qed_llh_set_ppfid_affinity() > > So export qed_llh and move the qed_iwarp to the rdma driver > > etc Got it, and makes sense to me. I get the point on single instance of function pointers being redundant. We will start work on the necessary redesign right away. Meanwhile you may see a few more critical fixes/small features coming from us which are already queued up internally on our end, which I hope can be accepted before we perform the changes discussed here. Thanks, Ariel > > Jason