On Tue, 27 Aug 2019 11:33:54 +0000 Parav Pandit <parav@xxxxxxxxxxxx> wrote: > > -----Original Message----- > > From: Cornelia Huck <cohuck@xxxxxxxxxx> > > Sent: Tuesday, August 27, 2019 4:54 PM > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > Cc: alex.williamson@xxxxxxxxxx; Jiri Pirko <jiri@xxxxxxxxxxxx>; > > kwankhede@xxxxxxxxxx; davem@xxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias > > > > On Tue, 27 Aug 2019 11:12:23 +0000 > > Parav Pandit <parav@xxxxxxxxxxxx> wrote: > > > > > > -----Original Message----- > > > > From: Cornelia Huck <cohuck@xxxxxxxxxx> > > > > Sent: Tuesday, August 27, 2019 3:54 PM > > > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > > > Cc: alex.williamson@xxxxxxxxxx; Jiri Pirko <jiri@xxxxxxxxxxxx>; > > > > kwankhede@xxxxxxxxxx; davem@xxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; > > > > linux- kernel@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx > > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias > > > > > > > > What about: > > > > > > > > * @get_alias_length: optional callback to specify length of the alias to > > create > > > > * Returns unsigned integer: length of the alias to be created, > > > > * 0 to not create an alias > > > > > > > Ack. > > > > > > > I also think it might be beneficial to add a device parameter here > > > > now (rather than later); that seems to be something that makes sense. > > > > > > > Without showing the use, it shouldn't be added. > > > > It just feels like an omission: Why should the vendor driver only be able to > > return one value here, without knowing which device it is for? > > If a driver supports different devices, it may have different requirements for > > them. > > > Sure. Lets first have this requirement to add it. > I am against adding this length field itself without an actual vendor use case, which is adding some complexity in code today. > But it was ok to have length field instead of bool. > > Lets not further add "no-requirement futuristic knobs" which hasn't shown its need yet. > When a vendor driver needs it, there is nothing prevents such addition. Frankly, I do not see how it adds complexity; the other callbacks have device arguments already, and the vendor driver is free to ignore it if it does not have a use for it. I'd rather add the argument before a possible future user tries weird hacks to allow multiple values, but I'll leave the decision to the maintainers.