linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] qed: Enable RDMA relaxed ordering
@ 2021-08-26 12:05 Shai Malin
  2021-08-26 13:06 ` Jason Gunthorpe
  0 siblings, 1 reply; 5+ messages in thread
From: Shai Malin @ 2021-08-26 12:05 UTC (permalink / raw)
  To: Leon Romanovsky, davem, kuba
  Cc: netdev, Ariel Elior, malin1024, RDMA mailing list, jgg

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
> > --- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> > +++ 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.

> 
> If we are talking about RDMA, the IB_ACCESS_RELAXED_ORDERING flag should
> be taken into account too.

Thanks! 
We will need to add both FW and driver support to enable it.

> 
> Thanks
> 
> > +
> >  	return qed_spq_post(p_hwfn, p_ent, NULL);
> >  }
> >
> > --
> > 2.22.0
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] qed: Enable RDMA relaxed ordering
  2021-08-26 12:05 [PATCH] qed: Enable RDMA relaxed ordering Shai Malin
@ 2021-08-26 13:06 ` Jason Gunthorpe
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2021-08-26 13:06 UTC (permalink / raw)
  To: Shai Malin
  Cc: Leon Romanovsky, davem, kuba, netdev, Ariel Elior, malin1024,
	RDMA mailing list

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?

Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] qed: Enable RDMA relaxed ordering
@ 2021-08-26 13:40 Shai Malin
  0 siblings, 0 replies; 5+ messages in thread
From: Shai Malin @ 2021-08-26 13:40 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, davem, kuba, netdev, Ariel Elior, malin1024,
	RDMA mailing list

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] qed: Enable RDMA relaxed ordering
  2021-08-23 11:52 ` Leon Romanovsky
@ 2021-08-23 13:33   ` Jason Gunthorpe
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2021-08-23 13:33 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Shai Malin, davem, kuba, netdev, aelior, malin1024, RDMA mailing list

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] qed: Enable RDMA relaxed ordering
       [not found] <20210822185448.12053-1-smalin@marvell.com>
@ 2021-08-23 11:52 ` Leon Romanovsky
  2021-08-23 13:33   ` Jason Gunthorpe
  0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2021-08-23 11:52 UTC (permalink / raw)
  To: Shai Malin, davem, kuba; +Cc: netdev, aelior, malin1024, RDMA mailing list

+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
> --- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
> +++ 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.

Thanks

> +
>  	return qed_spq_post(p_hwfn, p_ent, NULL);
>  }
>  
> -- 
> 2.22.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-26 13:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-26 12:05 [PATCH] qed: Enable RDMA relaxed ordering Shai Malin
2021-08-26 13:06 ` Jason Gunthorpe
  -- strict thread matches above, loose matches on Subject: below --
2021-08-26 13:40 Shai Malin
     [not found] <20210822185448.12053-1-smalin@marvell.com>
2021-08-23 11:52 ` Leon Romanovsky
2021-08-23 13:33   ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).