Re: [EXT] Re: [PATCH] qed: Enable RDMA relaxed ordering

[Leon Romanovsky <leon@kernel.org>]

On Mon, Aug 23, 2021 at 12:17:42PM -0300, Jason Gunthorpe wrote:
> On Mon, Aug 23, 2021 at 02:54:13PM +0000, Ariel Elior wrote:
> > > From: Jason Gunthorpe <jgg@ziepe.ca>
> > > Sent: Monday, August 23, 2021 4:34 PM
> > > To: Leon Romanovsky <leon@kernel.org>
> > > Cc: Shai Malin <smalin@marvell.com>; davem@davemloft.net;
> > > kuba@kernel.org; netdev@vger.kernel.org; Ariel Elior
> > > <aelior@marvell.com>; malin1024@gmail.com; RDMA mailing list <linux-
> > > rdma@vger.kernel.org>
> > > Subject: [EXT] Re: [PATCH] qed: Enable RDMA relaxed ordering
> > > 
> > > External Email
> > > 
> > > On Mon, Aug 23, 2021 at 02:52:21PM +0300, Leon Romanovsky wrote:
> > > > +RDMA
> > > >
> > > > Jakub, David
> > > >
> > > > Can we please ask that everything directly or indirectly related to
> > > > RDMA will be sent to linux-rdma@ too?
> > > >
> > > > On Sun, Aug 22, 2021 at 09:54:48PM +0300, Shai Malin wrote:
> > > > > Enable the RoCE and iWARP FW relaxed ordering.
> > > > >
> > > > > Signed-off-by: Ariel Elior <aelior@marvell.com>
> > > > > Signed-off-by: Shai Malin <smalin@marvell.com>
> > > > > drivers/net/ethernet/qlogic/qed/qed_rdma.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> > > > > b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> > > > > index 4f4b79250a2b..496092655f26 100644
> > > > > +++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> > > > > @@ -643,6 +643,8 @@ static int qed_rdma_start_fw(struct qed_hwfn
> > > *p_hwfn,
> > > > >  				    cnq_id);
> > > > >  	}
> > > > >
> > > > > +	p_params_header->relaxed_ordering = 1;
> > > >
> > > > Maybe it is only description that needs to be updated, but I would
> > > > expect to see call to pcie_relaxed_ordering_enabled() before setting
> > > > relaxed_ordering to always true.
> > > >
> > > > If we are talking about RDMA, the IB_ACCESS_RELAXED_ORDERING flag
> > > > should be taken into account too.
> > > 
> > > Why does this file even exist in netdev? This whole struct qed_rdma_ops
> > > mess looks like another mis-design to support out of tree modules??
> > > 
> > > Jason
> > 
> > Hi Jason,
> > qed_rdma_ops is not related to in tree / out of tree drivers. The qed is the
> > core module which is used by the protocol drivers which drive this type of nic:
> > qede, qedr, qedi and qedf for ethernet, rdma, iscsi and fcoe respectively.
> > qed mostly serves as a HW abstraction layer, hiding away the details of FW
> > interaction and device register usage, and may also hold Linux specific things
> > which are protocol agnostic, such as dcbx, sriov, debug data collection logic,
> > etc. qed interacts with the protocol drivers through ops structs for many
> > purposes (dcbx, ptp, sriov, etc). And also for rdma. It's just a way for us to
> > separate the modules in a clean way.
> 
> Delete the ops struct.
> 
> Move the RDMA functions to the RDMA module
> 
> Directly export the core functions needed to make that work
> 
> Two halfs of the same dirver do not and should not have an ops structure
> ABI between them.

Yea, I read drivers/net/ethernet/qlogic/qed/qed_rdma.c and have hard
time to believe that hiding RDMA objects and code from the RDMA community
can be counted as "a clean way".

Thanks

> 
> Jason