Re: [PATCH] qed: Enable RDMA relaxed ordering

[Shai Malin <smalin@marvell.com>]

On Thu, Aug 26, 2021 at 04:07:00PM +0030, Jason Gunthorpe wrote:
> On Thu, Aug 26, 2021 at 12:05:18PM +0000, Shai Malin wrote:
> > 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?
> >
> > In addition to all that was discussed regarding qed_rdma.c
> > and qed_rdma_ops - certainly, everything directly or indirectly
> > related to RDMA will be sent to linux-rdma.
> >
> > >
> > > 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.
> >
> > This change will only allow the FW to support relaxed ordering but it
> > will be enabled only if the device/root-complex/server supports
> > relaxed ordering.
> > The pcie_relaxed_ordering_enabled() is not needed in this case.
> 
> I'm confused, our RDMA model is not to blanket enable relaxed
> ordering, we set out rules that the driver has to follow when a
> relaxed ordering TLP can be issued:
>  - QP/CQ/etc internal queues - device decision based on correctness
>  - Kernel created MRs - always
>  - User created MRs - only if IB_ACCESS_RELAXED_ORDERING is set
> 
> So what does this flag do, and does it follow this model?

The flag was not following the model. It was supposed to enable the 
RDMA FW to use relaxed ordering for all the above cases as long as the 
device will allow it. 
Following Leon's comment, I understand that it is not allowed with 
user created MRs.

Our gap is that in order to control the per MR relaxed ordering we will 
need to update the FW (in addition to the driver changes).

> 
> Jason