mbox series

[v5,0/5] RDMA/mana_ib

Message ID 1694105559-9465-1-git-send-email-sharmaajay@linuxonhyperv.com (mailing list archive)
Headers show
Series RDMA/mana_ib | expand

Message

sharmaajay@linuxonhyperv.com Sept. 7, 2023, 4:52 p.m. UTC
From: Ajay Sharma <sharmaajay@microsoft.com>

Change from v4:
Send qp fatal error event to the context that
created the qp. Add lookup table for qp.

Ajay Sharma (5):
  RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
  RDMA/mana_ib : Register Mana IB  device with Management SW
  RDMA/mana_ib : Create adapter and Add error eq
  RDMA/mana_ib : Query adapter capabilities
  RDMA/mana_ib : Send event to qp

 drivers/infiniband/hw/mana/cq.c               |  12 +-
 drivers/infiniband/hw/mana/device.c           |  81 +++--
 drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
 drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
 drivers/infiniband/hw/mana/mr.c               |  42 ++-
 drivers/infiniband/hw/mana/qp.c               |  86 +++---
 drivers/infiniband/hw/mana/wq.c               |  21 +-
 .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
 drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
 include/net/mana/gdma.h                       |  16 +-
 10 files changed, 545 insertions(+), 258 deletions(-)

Comments

Leon Romanovsky Sept. 11, 2023, 12:32 p.m. UTC | #1
On Thu, Sep 07, 2023 at 09:52:34AM -0700, sharmaajay@linuxonhyperv.com wrote:
> From: Ajay Sharma <sharmaajay@microsoft.com>
> 
> Change from v4:
> Send qp fatal error event to the context that
> created the qp. Add lookup table for qp.
> 
> Ajay Sharma (5):
>   RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
>   RDMA/mana_ib : Register Mana IB  device with Management SW
>   RDMA/mana_ib : Create adapter and Add error eq
>   RDMA/mana_ib : Query adapter capabilities
>   RDMA/mana_ib : Send event to qp

I didn't look very deep into the series and has three very initial comments.
1. Please do git log drivers/infiniband/hw/mana/ and use same format for
commit messages.
2. Don't invent your own index-to-qp query mechanism in last patch and use xarray.
3. Once you decided to export mana_gd_register_device, it hinted me that
it is time to move to auxbus infrastructure.

Thanks

> 
>  drivers/infiniband/hw/mana/cq.c               |  12 +-
>  drivers/infiniband/hw/mana/device.c           |  81 +++--
>  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
>  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
>  drivers/infiniband/hw/mana/mr.c               |  42 ++-
>  drivers/infiniband/hw/mana/qp.c               |  86 +++---
>  drivers/infiniband/hw/mana/wq.c               |  21 +-
>  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
>  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
>  include/net/mana/gdma.h                       |  16 +-
>  10 files changed, 545 insertions(+), 258 deletions(-)
> 
> -- 
> 2.25.1
>
Ajay Sharma Sept. 11, 2023, 6:57 p.m. UTC | #2
I have updated the last patch to use xarray, will post the update patch. We currently use aux bus for ib device. Gd_register_device is firmware specific. All the patches use RDMA/mana_ib format which is aligned with drivers/infiniband/hw/mana/ .

Thanks

> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Monday, September 11, 2023 7:33 AM
> To: sharmaajay@linuxonhyperv.com
> Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe <jgg@ziepe.ca>; Dexuan
> Cui <decui@microsoft.com>; Wei Liu <wei.liu@kernel.org>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-
> rdma@vger.kernel.org; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Ajay Sharma <sharmaajay@microsoft.com>
> Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> 
> On Thu, Sep 07, 2023 at 09:52:34AM -0700, sharmaajay@linuxonhyperv.com
> wrote:
> > From: Ajay Sharma <sharmaajay@microsoft.com>
> >
> > Change from v4:
> > Send qp fatal error event to the context that created the qp. Add
> > lookup table for qp.
> >
> > Ajay Sharma (5):
> >   RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
> >   RDMA/mana_ib : Register Mana IB  device with Management SW
> >   RDMA/mana_ib : Create adapter and Add error eq
> >   RDMA/mana_ib : Query adapter capabilities
> >   RDMA/mana_ib : Send event to qp
> 
> I didn't look very deep into the series and has three very initial comments.
> 1. Please do git log drivers/infiniband/hw/mana/ and use same format for
> commit messages.
> 2. Don't invent your own index-to-qp query mechanism in last patch and use
> xarray.
> 3. Once you decided to export mana_gd_register_device, it hinted me that it is
> time to move to auxbus infrastructure.
> 
> Thanks
> 
> >
> >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
> >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> >  include/net/mana/gdma.h                       |  16 +-
> >  10 files changed, 545 insertions(+), 258 deletions(-)
> >
> > --
> > 2.25.1
> >
Leon Romanovsky Sept. 18, 2023, 8:29 a.m. UTC | #3
On Mon, Sep 11, 2023 at 06:57:21PM +0000, Ajay Sharma wrote:
> I have updated the last patch to use xarray, will post the update patch. We currently use aux bus for ib device. Gd_register_device is firmware specific. All the patches use RDMA/mana_ib format which is aligned with drivers/infiniband/hw/mana/ .

➜  kernel git:(wip/leon-for-rc) git l --no-merges drivers/infiniband/hw/mana/
2145328515c8 RDMA/mana_ib: Use v2 version of cfg_rx_steer_req to enable RX coalescing
89d42b8c85b4 RDMA/mana_ib: Fix a bug when the PF indicates more entries for registering memory on first packet
563ca0e9eab8 RDMA/mana_ib: Prevent array underflow in mana_ib_create_qp_raw()
3574cfdca285 RDMA/mana: Remove redefinition of basic u64 type
0266a177631d RDMA/mana_ib: Add a driver for Microsoft Azure Network Adapter

It is different format from presented here. You added extra space before ":"
and there is double space in one of the titles.

Regarding aux, I see it, but what confuses me is proliferation of terms
and various calls: device, client, adapter. My expectation is to see
more uniform methodology where IB is represented as device.

Thanks

> 
> Thanks
> 
> > -----Original Message-----
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Monday, September 11, 2023 7:33 AM
> > To: sharmaajay@linuxonhyperv.com
> > Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe <jgg@ziepe.ca>; Dexuan
> > Cui <decui@microsoft.com>; Wei Liu <wei.liu@kernel.org>; David S. Miller
> > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-
> > rdma@vger.kernel.org; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Ajay Sharma <sharmaajay@microsoft.com>
> > Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > 
> > On Thu, Sep 07, 2023 at 09:52:34AM -0700, sharmaajay@linuxonhyperv.com
> > wrote:
> > > From: Ajay Sharma <sharmaajay@microsoft.com>
> > >
> > > Change from v4:
> > > Send qp fatal error event to the context that created the qp. Add
> > > lookup table for qp.
> > >
> > > Ajay Sharma (5):
> > >   RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
> > >   RDMA/mana_ib : Register Mana IB  device with Management SW
> > >   RDMA/mana_ib : Create adapter and Add error eq
> > >   RDMA/mana_ib : Query adapter capabilities
> > >   RDMA/mana_ib : Send event to qp
> > 
> > I didn't look very deep into the series and has three very initial comments.
> > 1. Please do git log drivers/infiniband/hw/mana/ and use same format for
> > commit messages.
> > 2. Don't invent your own index-to-qp query mechanism in last patch and use
> > xarray.
> > 3. Once you decided to export mana_gd_register_device, it hinted me that it is
> > time to move to auxbus infrastructure.
> > 
> > Thanks
> > 
> > >
> > >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> > >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> > >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
> > >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> > >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> > >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> > >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> > >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> > >  include/net/mana/gdma.h                       |  16 +-
> > >  10 files changed, 545 insertions(+), 258 deletions(-)
> > >
> > > --
> > > 2.25.1
> > >
Ajay Sharma Oct. 16, 2023, 10:15 p.m. UTC | #4
I have sent v7 patch series with the space removed 

> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Monday, September 18, 2023 1:29 AM
> To: Ajay Sharma <sharmaajay@microsoft.com>
> Cc: sharmaajay@linuxonhyperv.com; Long Li <longli@microsoft.com>; Jason
> Gunthorpe <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>; Wei Liu
> <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; linux-rdma@vger.kernel.org; linux-
> hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> 
> On Mon, Sep 11, 2023 at 06:57:21PM +0000, Ajay Sharma wrote:
> > I have updated the last patch to use xarray, will post the update patch. We
> currently use aux bus for ib device. Gd_register_device is firmware specific. All
> the patches use RDMA/mana_ib format which is aligned with
> drivers/infiniband/hw/mana/ .
> 
> ➜  kernel git:(wip/leon-for-rc) git l --no-merges drivers/infiniband/hw/mana/
> 2145328515c8 RDMA/mana_ib: Use v2 version of cfg_rx_steer_req to enable
> RX coalescing
> 89d42b8c85b4 RDMA/mana_ib: Fix a bug when the PF indicates more entries
> for registering memory on first packet
> 563ca0e9eab8 RDMA/mana_ib: Prevent array underflow in
> mana_ib_create_qp_raw()
> 3574cfdca285 RDMA/mana: Remove redefinition of basic u64 type
> 0266a177631d RDMA/mana_ib: Add a driver for Microsoft Azure Network
> Adapter
> 
> It is different format from presented here. You added extra space before ":"
> and there is double space in one of the titles.
> 
I have removed the space

> Regarding aux, I see it, but what confuses me is proliferation of terms and
> various calls: device, client, adapter. My expectation is to see more uniform
> methodology where IB is represented as device.
> 

The adapter is a software construct. It is used as a container for resources. Client is used to distinguish between eth and ib.  Please note that these are terms used for different purposes on the management side. 

> Thanks
> 
> >
> > Thanks
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky <leon@kernel.org>
> > > Sent: Monday, September 11, 2023 7:33 AM
> > > To: sharmaajay@linuxonhyperv.com
> > > Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe <jgg@ziepe.ca>;
> > > Dexuan Cui <decui@microsoft.com>; Wei Liu <wei.liu@kernel.org>;
> > > David S. Miller <davem@davemloft.net>; Eric Dumazet
> > > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> > > <pabeni@redhat.com>; linux- rdma@vger.kernel.org;
> > > linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; Ajay Sharma <sharmaajay@microsoft.com>
> > > Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > >
> > > On Thu, Sep 07, 2023 at 09:52:34AM -0700,
> > > sharmaajay@linuxonhyperv.com
> > > wrote:
> > > > From: Ajay Sharma <sharmaajay@microsoft.com>
> > > >
> > > > Change from v4:
> > > > Send qp fatal error event to the context that created the qp. Add
> > > > lookup table for qp.
> > > >
> > > > Ajay Sharma (5):
> > > >   RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
> > > >   RDMA/mana_ib : Register Mana IB  device with Management SW
> > > >   RDMA/mana_ib : Create adapter and Add error eq
> > > >   RDMA/mana_ib : Query adapter capabilities
> > > >   RDMA/mana_ib : Send event to qp
> > >
> > > I didn't look very deep into the series and has three very initial comments.
> > > 1. Please do git log drivers/infiniband/hw/mana/ and use same format
> > > for commit messages.
> > > 2. Don't invent your own index-to-qp query mechanism in last patch
> > > and use xarray.
> > > 3. Once you decided to export mana_gd_register_device, it hinted me
> > > that it is time to move to auxbus infrastructure.
> > >
> > > Thanks
> > >
> > > >
> > > >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> > > >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> > > >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
> > > >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> > > >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> > > >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> > > >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> > > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> > > >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> > > >  include/net/mana/gdma.h                       |  16 +-
> > > >  10 files changed, 545 insertions(+), 258 deletions(-)
> > > >
> > > > --
> > > > 2.25.1
> > > >
Leon Romanovsky Oct. 17, 2023, 7:48 a.m. UTC | #5
On Mon, Oct 16, 2023 at 10:15:18PM +0000, Ajay Sharma wrote:
> I have sent v7 patch series with the space removed 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Monday, September 18, 2023 1:29 AM
> > To: Ajay Sharma <sharmaajay@microsoft.com>
> > Cc: sharmaajay@linuxonhyperv.com; Long Li <longli@microsoft.com>; Jason
> > Gunthorpe <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>; Wei Liu
> > <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> > <pabeni@redhat.com>; linux-rdma@vger.kernel.org; linux-
> > hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > 
> > On Mon, Sep 11, 2023 at 06:57:21PM +0000, Ajay Sharma wrote:
> > > I have updated the last patch to use xarray, will post the update patch. We
> > currently use aux bus for ib device. Gd_register_device is firmware specific. All
> > the patches use RDMA/mana_ib format which is aligned with
> > drivers/infiniband/hw/mana/ .
> > 
> > ➜  kernel git:(wip/leon-for-rc) git l --no-merges drivers/infiniband/hw/mana/
> > 2145328515c8 RDMA/mana_ib: Use v2 version of cfg_rx_steer_req to enable
> > RX coalescing
> > 89d42b8c85b4 RDMA/mana_ib: Fix a bug when the PF indicates more entries
> > for registering memory on first packet
> > 563ca0e9eab8 RDMA/mana_ib: Prevent array underflow in
> > mana_ib_create_qp_raw()
> > 3574cfdca285 RDMA/mana: Remove redefinition of basic u64 type
> > 0266a177631d RDMA/mana_ib: Add a driver for Microsoft Azure Network
> > Adapter
> > 
> > It is different format from presented here. You added extra space before ":"
> > and there is double space in one of the titles.
> > 
> I have removed the space
> 
> > Regarding aux, I see it, but what confuses me is proliferation of terms and
> > various calls: device, client, adapter. My expectation is to see more uniform
> > methodology where IB is represented as device.
> > 
> 
> The adapter is a software construct. It is used as a container for resources. 
> Client is used to distinguish between eth and ib.

Do you have multiple "adapters" in one ib/eth device?
If yes, at least for IB, it will be very unexpected to see.

Why do you have client and device when they are basically the same objects?

> Please note that these are terms used for different purposes on the management side.

We are discussing RDMA side of this series.

Thanks

> 
> > Thanks
> > 
> > >
> > > Thanks
> > >
> > > > -----Original Message-----
> > > > From: Leon Romanovsky <leon@kernel.org>
> > > > Sent: Monday, September 11, 2023 7:33 AM
> > > > To: sharmaajay@linuxonhyperv.com
> > > > Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe <jgg@ziepe.ca>;
> > > > Dexuan Cui <decui@microsoft.com>; Wei Liu <wei.liu@kernel.org>;
> > > > David S. Miller <davem@davemloft.net>; Eric Dumazet
> > > > <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> > > > <pabeni@redhat.com>; linux- rdma@vger.kernel.org;
> > > > linux-hyperv@vger.kernel.org; netdev@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; Ajay Sharma <sharmaajay@microsoft.com>
> > > > Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > > >
> > > > On Thu, Sep 07, 2023 at 09:52:34AM -0700,
> > > > sharmaajay@linuxonhyperv.com
> > > > wrote:
> > > > > From: Ajay Sharma <sharmaajay@microsoft.com>
> > > > >
> > > > > Change from v4:
> > > > > Send qp fatal error event to the context that created the qp. Add
> > > > > lookup table for qp.
> > > > >
> > > > > Ajay Sharma (5):
> > > > >   RDMA/mana_ib : Rename all mana_ib_dev type variables to mib_dev
> > > > >   RDMA/mana_ib : Register Mana IB  device with Management SW
> > > > >   RDMA/mana_ib : Create adapter and Add error eq
> > > > >   RDMA/mana_ib : Query adapter capabilities
> > > > >   RDMA/mana_ib : Send event to qp
> > > >
> > > > I didn't look very deep into the series and has three very initial comments.
> > > > 1. Please do git log drivers/infiniband/hw/mana/ and use same format
> > > > for commit messages.
> > > > 2. Don't invent your own index-to-qp query mechanism in last patch
> > > > and use xarray.
> > > > 3. Once you decided to export mana_gd_register_device, it hinted me
> > > > that it is time to move to auxbus infrastructure.
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> > > > >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> > > > >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
> > > > >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> > > > >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> > > > >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> > > > >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> > > > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> > > > >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> > > > >  include/net/mana/gdma.h                       |  16 +-
> > > > >  10 files changed, 545 insertions(+), 258 deletions(-)
> > > > >
> > > > > --
> > > > > 2.25.1
> > > > >
Ajay Sharma Oct. 17, 2023, 6:51 p.m. UTC | #6
> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Tuesday, October 17, 2023 12:48 AM
> To: Ajay Sharma <sharmaajay@microsoft.com>
> Cc: sharmaajay@linuxonhyperv.com; Long Li <longli@microsoft.com>; Jason
> Gunthorpe <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>; Wei Liu
> <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; linux-rdma@vger.kernel.org; linux-
> hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> 
> On Mon, Oct 16, 2023 at 10:15:18PM +0000, Ajay Sharma wrote:
> > I have sent v7 patch series with the space removed
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky <leon@kernel.org>
> > > Sent: Monday, September 18, 2023 1:29 AM
> > > To: Ajay Sharma <sharmaajay@microsoft.com>
> > > Cc: sharmaajay@linuxonhyperv.com; Long Li <longli@microsoft.com>;
> > > Jason Gunthorpe <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>;
> > > Wei Liu <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>;
> > > Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> > > linux-rdma@vger.kernel.org; linux- hyperv@vger.kernel.org;
> > > netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > >
> > > On Mon, Sep 11, 2023 at 06:57:21PM +0000, Ajay Sharma wrote:
> > > > I have updated the last patch to use xarray, will post the update
> > > > patch. We
> > > currently use aux bus for ib device. Gd_register_device is firmware
> > > specific. All the patches use RDMA/mana_ib format which is aligned
> > > with drivers/infiniband/hw/mana/ .
> > >
> > > ➜  kernel git:(wip/leon-for-rc) git l --no-merges
> > > drivers/infiniband/hw/mana/
> > > 2145328515c8 RDMA/mana_ib: Use v2 version of cfg_rx_steer_req to
> > > enable RX coalescing
> > > 89d42b8c85b4 RDMA/mana_ib: Fix a bug when the PF indicates more
> > > entries for registering memory on first packet
> > > 563ca0e9eab8 RDMA/mana_ib: Prevent array underflow in
> > > mana_ib_create_qp_raw()
> > > 3574cfdca285 RDMA/mana: Remove redefinition of basic u64 type
> > > 0266a177631d RDMA/mana_ib: Add a driver for Microsoft Azure Network
> > > Adapter
> > >
> > > It is different format from presented here. You added extra space before ":"
> > > and there is double space in one of the titles.
> > >
> > I have removed the space
> >
> > > Regarding aux, I see it, but what confuses me is proliferation of
> > > terms and various calls: device, client, adapter. My expectation is
> > > to see more uniform methodology where IB is represented as device.
> > >
> >
> > The adapter is a software construct. It is used as a container for resources.
> > Client is used to distinguish between eth and ib.
> 
> Do you have multiple "adapters" in one ib/eth device?
> If yes, at least for IB, it will be very unexpected to see.
> 
Adapter is IB specific and one per VF/pcie device. It's the handle that is passed
between the management and VM for book keeping.
> Why do you have client and device when they are basically the same objects?
> 
I am not sure which ones you are referring to specifically , can you please elaborate?

> > Please note that these are terms used for different purposes on the
> management side.
> 
> We are discussing RDMA side of this series.
> 
> Thanks
> 
> >
> > > Thanks
> > >
> > > >
> > > > Thanks
> > > >
> > > > > -----Original Message-----
> > > > > From: Leon Romanovsky <leon@kernel.org>
> > > > > Sent: Monday, September 11, 2023 7:33 AM
> > > > > To: sharmaajay@linuxonhyperv.com
> > > > > Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe
> > > > > <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>; Wei Liu
> > > > > <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>;
> > > > > Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > > > > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-
> > > > > rdma@vger.kernel.org; linux-hyperv@vger.kernel.org;
> > > > > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ajay
> > > > > Sharma <sharmaajay@microsoft.com>
> > > > > Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > > > >
> > > > > On Thu, Sep 07, 2023 at 09:52:34AM -0700,
> > > > > sharmaajay@linuxonhyperv.com
> > > > > wrote:
> > > > > > From: Ajay Sharma <sharmaajay@microsoft.com>
> > > > > >
> > > > > > Change from v4:
> > > > > > Send qp fatal error event to the context that created the qp.
> > > > > > Add lookup table for qp.
> > > > > >
> > > > > > Ajay Sharma (5):
> > > > > >   RDMA/mana_ib : Rename all mana_ib_dev type variables to
> mib_dev
> > > > > >   RDMA/mana_ib : Register Mana IB  device with Management SW
> > > > > >   RDMA/mana_ib : Create adapter and Add error eq
> > > > > >   RDMA/mana_ib : Query adapter capabilities
> > > > > >   RDMA/mana_ib : Send event to qp
> > > > >
> > > > > I didn't look very deep into the series and has three very initial
> comments.
> > > > > 1. Please do git log drivers/infiniband/hw/mana/ and use same
> > > > > format for commit messages.
> > > > > 2. Don't invent your own index-to-qp query mechanism in last
> > > > > patch and use xarray.
> > > > > 3. Once you decided to export mana_gd_register_device, it hinted
> > > > > me that it is time to move to auxbus infrastructure.
> > > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> > > > > >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> > > > > >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-----
> > > > > >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> > > > > >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> > > > > >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> > > > > >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> > > > > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> > > > > >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> > > > > >  include/net/mana/gdma.h                       |  16 +-
> > > > > >  10 files changed, 545 insertions(+), 258 deletions(-)
> > > > > >
> > > > > > --
> > > > > > 2.25.1
> > > > > >
Long Li Oct. 18, 2023, 12:51 a.m. UTC | #7
> > Do you have multiple "adapters" in one ib/eth device?
> > If yes, at least for IB, it will be very unexpected to see.
> >
> Adapter is IB specific and one per VF/pcie device. It's the handle that is passed
> between the management and VM for book keeping.

I think there are some confusions on the device.

There is one device, that supports both Ethernet and RDMA(through RoCE).

On the device, RoCE can't function without Ethernet. In the Linux, the RDMA device is modeled as auxiliary device to ethernet device.

The physical device exposes both Ethernet and RDMA management interfaces (as adapters) to the client (Linux kernel). When the MANA RDMA driver was first introduced, RAW QP was supported. There was no need to connect to the RDMA management interface. Many device hardware RDMA capabilities were hardcoded in the driver at the time. (There were some reviewers questioning this at the time).

With this patchset, we are connecting to the RDMA management interface on the device. This is for addressing the prior comments, and for supporting upcoming RC QP.

Thanks,

Long


> > Why do you have client and device when they are basically the same
> objects?
> >
> I am not sure which ones you are referring to specifically , can you please
> elaborate?
> 
> > > Please note that these are terms used for different purposes on the
> > management side.
> >
> > We are discussing RDMA side of this series.
> >
> > Thanks
> >
> > >
> > > > Thanks
> > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Leon Romanovsky <leon@kernel.org>
> > > > > > Sent: Monday, September 11, 2023 7:33 AM
> > > > > > To: sharmaajay@linuxonhyperv.com
> > > > > > Cc: Long Li <longli@microsoft.com>; Jason Gunthorpe
> > > > > > <jgg@ziepe.ca>; Dexuan Cui <decui@microsoft.com>; Wei Liu
> > > > > > <wei.liu@kernel.org>; David S. Miller <davem@davemloft.net>;
> > > > > > Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > > > > > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-
> > > > > > rdma@vger.kernel.org; linux-hyperv@vger.kernel.org;
> > > > > > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ajay
> > > > > > Sharma <sharmaajay@microsoft.com>
> > > > > > Subject: [EXTERNAL] Re: [Patch v5 0/5] RDMA/mana_ib
> > > > > >
> > > > > > On Thu, Sep 07, 2023 at 09:52:34AM -0700,
> > > > > > sharmaajay@linuxonhyperv.com
> > > > > > wrote:
> > > > > > > From: Ajay Sharma <sharmaajay@microsoft.com>
> > > > > > >
> > > > > > > Change from v4:
> > > > > > > Send qp fatal error event to the context that created the qp.
> > > > > > > Add lookup table for qp.
> > > > > > >
> > > > > > > Ajay Sharma (5):
> > > > > > >   RDMA/mana_ib : Rename all mana_ib_dev type variables to
> > mib_dev
> > > > > > >   RDMA/mana_ib : Register Mana IB  device with Management SW
> > > > > > >   RDMA/mana_ib : Create adapter and Add error eq
> > > > > > >   RDMA/mana_ib : Query adapter capabilities
> > > > > > >   RDMA/mana_ib : Send event to qp
> > > > > >
> > > > > > I didn't look very deep into the series and has three very
> > > > > > initial
> > comments.
> > > > > > 1. Please do git log drivers/infiniband/hw/mana/ and use same
> > > > > > format for commit messages.
> > > > > > 2. Don't invent your own index-to-qp query mechanism in last
> > > > > > patch and use xarray.
> > > > > > 3. Once you decided to export mana_gd_register_device, it
> > > > > > hinted me that it is time to move to auxbus infrastructure.
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > > >
> > > > > > >  drivers/infiniband/hw/mana/cq.c               |  12 +-
> > > > > > >  drivers/infiniband/hw/mana/device.c           |  81 +++--
> > > > > > >  drivers/infiniband/hw/mana/main.c             | 288 +++++++++++++-
> ----
> > > > > > >  drivers/infiniband/hw/mana/mana_ib.h          | 102 ++++++-
> > > > > > >  drivers/infiniband/hw/mana/mr.c               |  42 ++-
> > > > > > >  drivers/infiniband/hw/mana/qp.c               |  86 +++---
> > > > > > >  drivers/infiniband/hw/mana/wq.c               |  21 +-
> > > > > > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 152 +++++----
> > > > > > >  drivers/net/ethernet/microsoft/mana/mana_en.c |   3 +
> > > > > > >  include/net/mana/gdma.h                       |  16 +-
> > > > > > >  10 files changed, 545 insertions(+), 258 deletions(-)
> > > > > > >
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >