Thread (8 messages) 8 messages, 3 authors, 2021-08-25

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 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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help