RE: [EXT] Re: [PATCH] qed: Enable RDMA relaxed ordering
From: Ariel Elior <hidden>
Date: 2021-08-25 09:37:09
Also in:
netdev
-----Original Message----- From: Jason Gunthorpe <jgg@ziepe.ca> Sent: Tuesday, August 24, 2021 10:42 PM To: Ariel Elior <redacted> Cc: Leon Romanovsky <leon@kernel.org>; Shai Malin [off-list ref]; davem@davemloft.net; kuba@kernel.org; netdev@vger.kernel.org; malin1024@gmail.com; RDMA mailing list <linux- rdma@vger.kernel.org> Subject: Re: [EXT] Re: [PATCH] qed: Enable RDMA relaxed ordering On Tue, Aug 24, 2021 at 07:16:41PM +0000, Ariel Elior wrote:quoted
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 noproblem 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.quoted
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