> 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@xxxxxxxxx> > > > > .../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 know Parav is looking at the general problem of how to customize > > > what aux devices are created, that may be a better fit for this. > > > > > > Can you remove the devlink parts to make progress? > > > > It is important since otherwise the customer will have no way to use RoCEv2 on > this device. > > I'm not saying to not having it eventually, I'm just getting tired of looking at 23 > patches. You can argue it out after > Since this device cannot do concurrent protocols per function, is having a driver specific devlink to switch between the 2 also a NACK? There is something similar in another PCI driver. https://www.kernel.org/doc/html/latest/networking/devlink/qed.html Can we not adapt to Parav's solution (if it's a good fit) when it becomes available? Shiraz
