netdev - Re: [RFC PATCH v5 17/27] qedn: Add qedn probe

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: <YKUvWqvlZNGJqBCy@unreal>
Date:   Wed, 19 May 2021 18:31:38 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Shai Malin <malin1024@...il.com>
Cc:     Shai Malin <smalin@...vell.com>, netdev@...r.kernel.org,
        linux-nvme@...ts.infradead.org, davem@...emloft.net,
        kuba@...nel.org, sagi@...mberg.me, hch@....de, axboe@...com,
        kbusch@...nel.org, Ariel Elior <aelior@...vell.com>,
        Michal Kalderon <mkalderon@...vell.com>, okulkarni@...vell.com,
        pkushwaha@...vell.com, Dean Balandin <dbalandin@...vell.com>
Subject: Re: [RFC PATCH v5 17/27] qedn: Add qedn probe

On Wed, May 19, 2021 at 05:29:32PM +0300, Shai Malin wrote:
> On Wed, 19 May 2021 at 15:31, Leon Romanovsky wrote:
> > On Wed, May 19, 2021 at 02:13:30PM +0300, Shai Malin wrote:
> > > This patch introduces the functionality of loading and unloading
> > > physical function.
> > > qedn_probe() loads the offload device PF(physical function), and
> > > initialize the HW and the FW with the PF parameters using the
> > > HW ops->qed_nvmetcp_ops, which are similar to other "qed_*_ops" which
> > > are used by the qede, qedr, qedf and qedi device drivers.
> > > qedn_remove() unloads the offload device PF, re-initialize the HW and
> > > the FW with the PF parameters.
> > >
> > > The struct qedn_ctx is per PF container for PF-specific attributes and
> > > resources.
> > >
> > > Acked-by: Igor Russkikh <irusskikh@...vell.com>
> > > Signed-off-by: Dean Balandin <dbalandin@...vell.com>
> > > Signed-off-by: Prabhakar Kushwaha <pkushwaha@...vell.com>
> > > Signed-off-by: Omkar Kulkarni <okulkarni@...vell.com>
> > > Signed-off-by: Michal Kalderon <mkalderon@...vell.com>
> > > Signed-off-by: Ariel Elior <aelior@...vell.com>
> > > Signed-off-by: Shai Malin <smalin@...vell.com>
> > > Reviewed-by: Hannes Reinecke <hare@...e.de>
> > > ---
> > >  drivers/nvme/hw/Kconfig          |   1 +
> > >  drivers/nvme/hw/qedn/qedn.h      |  35 +++++++
> > >  drivers/nvme/hw/qedn/qedn_main.c | 159 ++++++++++++++++++++++++++++++-
> > >  3 files changed, 190 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/nvme/hw/Kconfig b/drivers/nvme/hw/Kconfig
> > > index 374f1f9dbd3d..91b1bd6f07d8 100644
> > > --- a/drivers/nvme/hw/Kconfig
> > > +++ b/drivers/nvme/hw/Kconfig
> > > @@ -2,6 +2,7 @@
> > >  config NVME_QEDN
> > >       tristate "Marvell NVM Express over Fabrics TCP offload"
> > >       depends on NVME_TCP_OFFLOAD
> > > +     select QED_NVMETCP
> > >       help
> > >         This enables the Marvell NVMe TCP offload support (qedn).
> > >
> > > diff --git a/drivers/nvme/hw/qedn/qedn.h b/drivers/nvme/hw/qedn/qedn.h
> > > index bcd0748a10fd..f13073afbced 100644
> > > --- a/drivers/nvme/hw/qedn/qedn.h
> > > +++ b/drivers/nvme/hw/qedn/qedn.h
> > > @@ -6,14 +6,49 @@
> > >  #ifndef _QEDN_H_
> > >  #define _QEDN_H_
> > >
> > > +#include <linux/qed/qed_if.h>
> > > +#include <linux/qed/qed_nvmetcp_if.h>
> > > +
> > >  /* Driver includes */
> > >  #include "../../host/tcp-offload.h"
> > >
> > > +#define QEDN_MAJOR_VERSION           8
> > > +#define QEDN_MINOR_VERSION           62
> > > +#define QEDN_REVISION_VERSION                10
> > > +#define QEDN_ENGINEERING_VERSION     0
> > > +#define DRV_MODULE_VERSION __stringify(QEDE_MAJOR_VERSION) "."       \
> > > +             __stringify(QEDE_MINOR_VERSION) "."             \
> > > +             __stringify(QEDE_REVISION_VERSION) "."          \
> > > +             __stringify(QEDE_ENGINEERING_VERSION)
> > > +
> >
> > This driver module version is not used in this series and more
> > important the module version have no meaning in upstream at all
> > and the community strongly against addition of new such code.
> 
> Will be fixed.
> 
> >
> > >  #define QEDN_MODULE_NAME "qedn"
> >
> > And the general note, it will be great if you convert your probe/remove
> > flows to use auxiliary bus like other drivers that cross subsystems.
> 
> qedn is simply fitting in with the existing design of qed/qede/qedr/qedf/qedi.

I know.

> Changing the entire multi-protocol design to auxiliary bus is being studied.

It will be required to do at some point of time.

Thanks