Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias

[Alex Williamson <alex.williamson@redhat.com>]

On Fri, 8 Nov 2019 15:30:59 +0000
Parav Pandit <parav@mellanox.com> wrote:

> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Friday, November 8, 2019 9:28 AM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; davem@davemloft.net;
> > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed
> > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri
> > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org
> > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty alias
> > 
> > On Fri, 8 Nov 2019 15:10:42 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Friday, November 8, 2019 7:46 AM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; davem@davemloft.net;
> > > > kvm@vger.kernel.org; netdev@vger.kernel.org; Saeed Mahameed
> > > > <saeedm@mellanox.com>; kwankhede@nvidia.com; leon@kernel.org; Jiri
> > > > Pirko <jiri@mellanox.com>; linux-rdma@vger.kernel.org
> > > > Subject: Re: [PATCH net-next 19/19] mtty: Optionally support mtty
> > > > alias
> > > >
> > > > On Thu,  7 Nov 2019 10:08:34 -0600
> > > > Parav Pandit <parav@mellanox.com> wrote:
> > > >  
> > > > > Provide a module parameter to set alias length to optionally
> > > > > generate mdev alias.
> > > > >
> > > > > Example to request mdev alias.
> > > > > $ modprobe mtty alias_length=12
> > > > >
> > > > > Make use of mtty_alias() API when alias_length module parameter is  
> > set.  
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > > ---
> > > > >  samples/vfio-mdev/mtty.c | 13 +++++++++++++
> > > > >  1 file changed, 13 insertions(+)  
> > > >
> > > > If you already have code using the alias interface, you probably
> > > > don't need to add it to the sample driver here. Especially as the
> > > > alias looks kind of pointless here.  
> > >
> > > It is pointless.
> > > Alex point when we ran through the series in August, was, QA should be  
> > able to do cover coverage of mdev_core where there is mdev collision and
> > mdev_create() can fail.  
> > > And QA should be able to set alias length to be short to 1 or 2 letters to  
> > trigger it.  
> > > Hence this patch was added.  
> > 
> > If we want this for testing purposes, that should be spelled out explicitly (the
> > above had already dropped from my cache). Even better if we had
> > something in actual test infrastructure.  
> 
> What else purpose sample driver has other than getting reference on how to use API? :-)

Yup, personally I still find the ROI for this worthwhile.  It gives us a
mechanism to test aliases, and particularly alias collisions, without
special hardware, as well as providing an example of the API.

FWIW, there will be merge conflicts with the alias support in this
series versus the mdev parent ops refactoring to allow class specific
device ops.  As the use of the alias support solidifies we can revisit
which branch we want to use to merge it upstream.  Thanks,

Alex