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

[Parav Pandit <parav@nvidia.com>]

> From: Parav Pandit <parav@nvidia.com>
> Sent: Tuesday, April 13, 2021 12:38 AM
> 
> > From: Saleem, Shiraz <shiraz.saleem@intel.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@intel.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
This was copy paste error from my command #1.
It should be "roce eth" should be "iwarp eth".
This is because in step #3, iwarp was enabled.

>   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.