netdev - RE: [PATCH v4 05/23] ice: Add devlink params support

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: <BY5PR12MB43221FA2A6295C9CF23C798DDC709@BY5PR12MB4322.namprd12.prod.outlook.com>
Date:   Mon, 12 Apr 2021 19:07:51 +0000
From:   Parav Pandit <parav@...dia.com>
To:     "Saleem, Shiraz" <shiraz.saleem@...el.com>,
        Jason Gunthorpe <jgg@...dia.com>
CC:     "dledford@...hat.com" <dledford@...hat.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "Lacombe, John S" <john.s.lacombe@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Ertman, David M" <david.m.ertman@...el.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "Hefty, Sean" <sean.hefty@...el.com>,
        "Keller, Jacob E" <jacob.e.keller@...el.com>
Subject: RE: [PATCH v4 05/23] ice: Add devlink params support



> From: Saleem, Shiraz <shiraz.saleem@...el.com>
> Sent: Monday, April 12, 2021 8:21 PM
> 
> > Subject: Re: [PATCH v4 05/23] ice: Add devlink params support
> >
> > On Wed, Apr 07, 2021 at 08:58:25PM +0000, Saleem, Shiraz wrote:
> > > > Subject: Re: [PATCH v4 05/23] ice: Add devlink params support
> > > >
> > > > On Tue, Apr 06, 2021 at 04:01:07PM -0500, Shiraz Saleem wrote:
> > > > > Add a new generic runtime devlink parameter 'rdma_protocol'
> > > > > and use it in ice PCI driver. Configuration changes result in
> > > > > unplugging the auxiliary RDMA device and re-plugging it with
> > > > > updated values for irdma auxiiary driver to consume at
> > > > > drv.probe()
> > > > >
> > > > > Signed-off-by: Shiraz Saleem <shiraz.saleem@...el.com>
> > > > >  .../networking/devlink/devlink-params.rst          |  6 ++
> > > > >  Documentation/networking/devlink/ice.rst           | 13 +++
> > > > >  drivers/net/ethernet/intel/ice/ice_devlink.c       | 92
> > +++++++++++++++++++++-
> > > > >  drivers/net/ethernet/intel/ice/ice_devlink.h       |  5 ++
> > > > >  drivers/net/ethernet/intel/ice/ice_main.c          |  2 +
> > > > >  include/net/devlink.h                              |  4 +
> > > > >  net/core/devlink.c                                 |  5 ++
> > > > >  7 files changed, 125 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/networking/devlink/devlink-params.rst
> > > > > b/Documentation/networking/devlink/devlink-params.rst
> > > > > index 54c9f10..0b454c3 100644
> > > > > +++ b/Documentation/networking/devlink/devlink-params.rst
> > > > > @@ -114,3 +114,9 @@ own name.
> > > > >         will NACK any attempt of other host to reset the device. This
> parameter
> > > > >         is useful for setups where a device is shared by different hosts,
> such
> > > > >         as multi-host setup.
> > > > > +   * - ``rdma_protocol``
> > > > > +     - string
> > > > > +     - Selects the RDMA protocol selected for multi-protocol devices.
> > > > > +        - ``iwarp`` iWARP
> > > > > +	- ``roce`` RoCE
> > > > > +	- ``ib`` Infiniband
> > > >
> > > > I'm still not sure this belongs in devlink.
> > >
> > > I believe you suggested we use devlink for protocol switch.
> >
> > Yes, devlink is the right place, but selecting a *single* protocol
> > doesn't seem right, or general enough.
> >
> > Parav is talking about generic ways to customize the aux devices
> > created and that would seem to serve the same function as this.
> 
> Is there an RFC or something posted for us to look at?
I do not have polished RFC content ready yet.
But coping the full config sequence snippet from the internal draft (changed for ice example) here as I like to discuss with you in this context.

# (1) show auxiliary device types supported by a given devlink device.
# applies to pci pf,vf,sf. (in general at devlink instance).
$ devlink dev auxdev show pci/0000:06.00.0
pci/0000:06.00.0:
  current:
    roce eth
  new:
  supported:
    roce eth iwarp

# (2) enable iwarp and ethernet type of aux devices and disable roce.
$ devlink dev auxdev set pci/0000:06:00.0 roce off iwarp on

# (3) now see which aux devices will be enable on next reload.
$ devlink dev auxdev show pci/0000:06:00.0
pci/0000:06:00.0:
  current:
    roce eth
  new:
    eth iwarp
  supported:
    roce eth iwarp

# (4) now reload the device and see which aux devices are created.
At this point driver undergoes reconfig for removal of roce and adding iwarp.
$ devlink reload pci/0000:06:00.0

# (5) verify which are the aux devices now activated.
$ devlink dev auxdev show pci/0000:06:00.0
pci/0000:06:00.0:
  current:
    roce eth
  new:
  supported:
    roce eth iwarp

Above 'new' section is only shown when its set. (similar to devlink resource).
In command two vendor driver can fail the call when iwarp and roce both enabled by user.
mlx5_core doesn't have iwarp, but its vdpa type of device. And for other cases we just want to enable roce disabling eth and vdpa.