netdev - Re: [RFC PATCH v5 01/16] RDMA/irdma: Add driver framework definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200421004628.GQ26002@ziepe.ca>
Date:   Mon, 20 Apr 2020 21:46:28 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     "Saleem, Shiraz" <shiraz.saleem@...el.com>
Cc:     Leon Romanovsky <leon@...nel.org>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "Ismail, Mustafa" <mustafa.ismail@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>
Subject: Re: [RFC PATCH v5 01/16] RDMA/irdma: Add driver framework definitions

On Tue, Apr 21, 2020 at 12:23:45AM +0000, Saleem, Shiraz wrote:
> > Subject: Re: [RFC PATCH v5 01/16] RDMA/irdma: Add driver framework
> > definitions
> > 
> > On Fri, Apr 17, 2020 at 10:12:36AM -0700, Jeff Kirsher wrote:
> > > From: Mustafa Ismail <mustafa.ismail@...el.com>
> > >
> > > Register irdma as a virtbus driver capable of supporting virtbus
> > > devices from multi-generation RDMA capable Intel HW. Establish the
> > > interface with all supported netdev peer drivers and initialize HW.
> > >
> > > Signed-off-by: Mustafa Ismail <mustafa.ismail@...el.com>
> > > Signed-off-by: Shiraz Saleem <shiraz.saleem@...el.com>
> > >  drivers/infiniband/hw/irdma/i40iw_if.c | 228 ++++++++++
> > > drivers/infiniband/hw/irdma/irdma_if.c | 449 ++++++++++++++++++
> > >  drivers/infiniband/hw/irdma/main.c     | 573 +++++++++++++++++++++++
> > >  drivers/infiniband/hw/irdma/main.h     | 599 +++++++++++++++++++++++++
> > >  4 files changed, 1849 insertions(+)
> > >  create mode 100644 drivers/infiniband/hw/irdma/i40iw_if.c
> > >  create mode 100644 drivers/infiniband/hw/irdma/irdma_if.c
> > >  create mode 100644 drivers/infiniband/hw/irdma/main.c
> > >  create mode 100644 drivers/infiniband/hw/irdma/main.h
> > >
> > 
> > I didn't look in too much details, but three things caught my attention immediately:
> > 1. Existence of ARP cache management logic in RDMA driver.
> 
> Our HW has an independent ARP table for the rdma block. 
> driver needs to add an ARP table entry via an rdma admin
> queue command before QP transitions to RTS.
> 
> > 2. Extensive use of dev_*() prints while we have ibdev_*() prints
> The ib device object is not available till the end of the device init
> similarly its unavailable early on in device deinit flows. So dev_*
> is all we can use in those places.

hns guys were thinking about changing this. It looks fine to just move
the name assignment to the device allocation, then we don't have this
weirdness

Alternatively, you could do as netdev does and have a special name
string when the name is NULL

Either way, I feel like this should be fixed up it is very fragile to
have two different print functions running around.

Jason