On Fri, 23 Aug 2019 08:14:39 +0000 Parav Pandit <pa...@mellanox.com> wrote:
> Hi Alex, > > > > -----Original Message----- > > From: Jiri Pirko <j...@resnulli.us> > > Sent: Friday, August 23, 2019 1:42 PM > > To: Parav Pandit <pa...@mellanox.com> > > Cc: Alex Williamson <alex.william...@redhat.com>; Jiri Pirko > > <j...@mellanox.com>; David S . Miller <da...@davemloft.net>; Kirti > > Wankhede <kwankh...@nvidia.com>; Cornelia Huck <coh...@redhat.com>; > > k...@vger.kernel.org; linux-kernel@vger.kernel.org; cjia <c...@nvidia.com>; > > net...@vger.kernel.org > > Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core > > > > Thu, Aug 22, 2019 at 03:33:30PM CEST, pa...@mellanox.com wrote: > > > > > > > > >> -----Original Message----- > > >> From: Jiri Pirko <j...@resnulli.us> > > >> Sent: Thursday, August 22, 2019 5:50 PM > > >> To: Parav Pandit <pa...@mellanox.com> > > >> Cc: Alex Williamson <alex.william...@redhat.com>; Jiri Pirko > > >> <j...@mellanox.com>; David S . Miller <da...@davemloft.net>; Kirti > > >> Wankhede <kwankh...@nvidia.com>; Cornelia Huck > > <coh...@redhat.com>; > > >> k...@vger.kernel.org; linux-kernel@vger.kernel.org; cjia > > >> <c...@nvidia.com>; net...@vger.kernel.org > > >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core > > >> > > >> Thu, Aug 22, 2019 at 12:04:02PM CEST, pa...@mellanox.com wrote: > > >> > > > >> > > > >> >> -----Original Message----- > > >> >> From: Jiri Pirko <j...@resnulli.us> > > >> >> Sent: Thursday, August 22, 2019 3:28 PM > > >> >> To: Parav Pandit <pa...@mellanox.com> > > >> >> Cc: Alex Williamson <alex.william...@redhat.com>; Jiri Pirko > > >> >> <j...@mellanox.com>; David S . Miller <da...@davemloft.net>; Kirti > > >> >> Wankhede <kwankh...@nvidia.com>; Cornelia Huck > > >> <coh...@redhat.com>; > > >> >> k...@vger.kernel.org; linux-kernel@vger.kernel.org; cjia > > >> >> <c...@nvidia.com>; net...@vger.kernel.org > > >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core > > >> >> > > >> >> Thu, Aug 22, 2019 at 11:42:13AM CEST, pa...@mellanox.com wrote: > > >> >> > > > >> >> > > > >> >> >> -----Original Message----- > > >> >> >> From: Jiri Pirko <j...@resnulli.us> > > >> >> >> Sent: Thursday, August 22, 2019 2:59 PM > > >> >> >> To: Parav Pandit <pa...@mellanox.com> > > >> >> >> Cc: Alex Williamson <alex.william...@redhat.com>; Jiri Pirko > > >> >> >> <j...@mellanox.com>; David S . Miller <da...@davemloft.net>; > > >> >> >> Kirti Wankhede <kwankh...@nvidia.com>; Cornelia Huck > > >> >> <coh...@redhat.com>; > > >> >> >> k...@vger.kernel.org; linux-kernel@vger.kernel.org; cjia > > >> >> >> <c...@nvidia.com>; net...@vger.kernel.org > > >> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core > > >> >> >> > > >> >> >> Wed, Aug 21, 2019 at 08:23:17AM CEST, pa...@mellanox.com wrote: > > >> >> >> > > > >> >> >> > > > >> >> >> >> -----Original Message----- > > >> >> >> >> From: Alex Williamson <alex.william...@redhat.com> > > >> >> >> >> Sent: Wednesday, August 21, 2019 10:56 AM > > >> >> >> >> To: Parav Pandit <pa...@mellanox.com> > > >> >> >> >> Cc: Jiri Pirko <j...@mellanox.com>; David S . Miller > > >> >> >> >> <da...@davemloft.net>; Kirti Wankhede > > >> >> >> >> <kwankh...@nvidia.com>; Cornelia Huck <coh...@redhat.com>; > > >> >> >> >> k...@vger.kernel.org; linux-kernel@vger.kernel.org; cjia > > >> >> >> >> <c...@nvidia.com>; net...@vger.kernel.org > > >> >> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev > > >> >> >> >> core > > >> >> >> >> > > >> >> >> >> > > > > Just an example of the alias, not proposing how it's > > >> >> >> >> > > > > set. > > >> >> >> >> > > > > In fact, proposing that the user does not set it, > > >> >> >> >> > > > > mdev-core provides one > > >> >> >> >> > > automatically. > > >> >> >> >> > > > > > > >> >> >> >> > > > > > > Since there seems to be some prefix overhead, as > > >> >> >> >> > > > > > > I ask about above in how many characters we > > >> >> >> >> > > > > > > actually have to work with in IFNAMESZ, maybe we > > >> >> >> >> > > > > > > start with > > >> >> >> >> > > > > > > 8 characters (matching your "index" namespace) > > >> >> >> >> > > > > > > and expand as necessary for > > >> >> >> disambiguation. > > >> >> >> >> > > > > > > If we can eliminate overhead in IFNAMESZ, let's > > >> >> >> >> > > > > > > start with > > >> 12. > > >> >> >> >> > > > > > > Thanks, > > >> >> >> >> > > > > > > > > >> >> >> >> > > > > > If user is going to choose the alias, why does it > > >> >> >> >> > > > > > have to be limited to > > >> >> >> >> sha1? > > >> >> >> >> > > > > > Or you just told it as an example? > > >> >> >> >> > > > > > > > >> >> >> >> > > > > > It can be an alpha-numeric string. > > >> >> >> >> > > > > > > >> >> >> >> > > > > No, I'm proposing a different solution where > > >> >> >> >> > > > > mdev-core creates an alias based on an abbreviated > > >> >> >> >> > > > > sha1. The user does not provide the > > >> >> >> >> alias. > > >> >> >> >> > > > > > > >> >> >> >> > > > > > Instead of mdev imposing number of characters on > > >> >> >> >> > > > > > the alias, it should be best > > >> >> >> >> > > > > left to the user. > > >> >> >> >> > > > > > Because in future if netdev improves on the naming > > >> >> >> >> > > > > > scheme, mdev will be > > >> >> >> >> > > > > limiting it, which is not right. > > >> >> >> >> > > > > > So not restricting alias size seems right to me. > > >> >> >> >> > > > > > User configuring mdev for networking devices in a > > >> >> >> >> > > > > > given kernel knows what > > >> >> >> >> > > > > user is doing. > > >> >> >> >> > > > > > So user can choose alias name size as it finds > > >> >> >> >> > > > > > suitable. > > >> >> >> >> > > > > > > >> >> >> >> > > > > That's not what I'm proposing, please read again. > > >> >> >> >> > > > > Thanks, > > >> >> >> >> > > > > > >> >> >> >> > > > I understood your point. But mdev doesn't know how > > >> >> >> >> > > > user is going to use > > >> >> >> >> > > udev/systemd to name the netdev. > > >> >> >> >> > > > So even if mdev chose to pick 12 characters, it could > > >> >> >> >> > > > result in > > >> >> collision. > > >> >> >> >> > > > Hence the proposal to provide the alias by the user, > > >> >> >> >> > > > as user know the best > > >> >> >> >> > > policy for its use case in the environment its using. > > >> >> >> >> > > > So 12 character sha1 method will still work by user. > > >> >> >> >> > > > > >> >> >> >> > > Haven't you already provided examples where certain > > >> >> >> >> > > drivers or subsystems have unique netdev prefixes? If > > >> >> >> >> > > mdev provides a unique alias within the subsystem, > > >> >> >> >> > > couldn't we simply define a netdev prefix for the mdev > > >> >> >> >> > > subsystem and avoid all other collisions? I'm not in > > >> >> >> >> > > favor of the user providing both a uuid and an > > >> >> >> >> > > alias/instance. Thanks, > > >> >> >> >> > > > > >> >> >> >> > For a given prefix, say ens2f0, can two UUID->sha1 first 9 > > >> >> >> >> > characters have > > >> >> >> >> collision? > > >> >> >> >> > > >> >> >> >> I think it would be a mistake to waste so many chars on a > > >> >> >> >> prefix, but > > >> >> >> >> 9 characters of sha1 likely wouldn't have a collision before > > >> >> >> >> we have 10s of thousands of devices. Thanks, > > >> >> >> >> > > >> >> >> >> Alex > > >> >> >> > > > >> >> >> >Jiri, Dave, > > >> >> >> >Are you ok with it for devlink/netdev part? > > >> >> >> >Mdev core will create an alias from a UUID. > > >> >> >> > > > >> >> >> >This will be supplied during devlink port attr set such as, > > >> >> >> > > > >> >> >> >devlink_port_attrs_mdev_set(struct devlink_port *port, const > > >> >> >> >char *mdev_alias); > > >> >> >> > > > >> >> >> >This alias is used to generate representor netdev's > > >> >> >> >phys_port_name. > > >> >> >> >This alias from the mdev device's sysfs will be used by the > > >> >> >> >udev/systemd to > > >> >> >> generate predicable netdev's name. > > >> >> >> >Example: enm<mdev_alias_first_12_chars> > > >> >> >> > > >> >> >> What happens in unlikely case of 2 UUIDs collide? > > >> >> >> > > >> >> >Since users sees two devices with same phys_port_name, user > > >> >> >should destroy > > >> >> recently created mdev and recreate mdev with different UUID? > > >> >> > > >> >> Driver should make sure phys port name wont collide, > > >> >So when mdev creation is initiated, mdev core calculates the alias > > >> >and if there > > >> is any other mdev with same alias exist, it returns -EEXIST error > > >> before progressing further. > > >> >This way user will get to know upfront in event of collision before > > >> >the mdev > > >> device gets created. > > >> >How about that? > > >> > > >> Sounds fine to me. Now the question is how many chars do we want to have. > > >> > > >12 characters from Alex's suggestion similar to git? > > > > Ok. > > > > Can you please confirm this scheme looks good now? I like to get patches > started. My only concern is your comment that in the event of an abbreviated sha1 collision (as exceptionally rare as that might be at 12-chars), we'd fail the device create, while my original suggestion was that vfio-core would add an extra character to the alias. For non-networking devices, the sha1 is unnecessary, so the extension behavior seems preferred. The user is only responsible to provide a unique uuid. Perhaps the failure behavior could be applied based on the mdev device_api. A module option on mdev to specify the default number of alias chars would also be useful for testing so that we can set it low enough to validate the collision behavior. Thanks, Alex > > >> >> in this case that it does > > >> >> not provide 2 same attrs for 2 different ports. > > >> >> Hmm, so the order of creation matters. That is not good. > > >> >> > > >> >> >> > > >> >> >> >I took Ethernet mdev as an example. > > >> >> >> >New prefix 'm' stands for mediated device. > > >> >> >> >Remaining 12 characters are first 12 chars of the mdev alias. > > >> >> >> > > >> >> >> Does this resolve the identification of devlink port representor? > > >> >> >Not sure if I understood your question correctly, attemping to > > >> >> >answer > > >> below. > > >> >> >phys_port_name of devlink port is defined by the first 12 > > >> >> >characters of mdev > > >> >> alias. > > >> >> >> I assume you want to use the same 12(or so) chars, don't you? > > >> >> >Mdev's netdev will also use the same mdev alias from the sysfs to > > >> >> >rename > > >> >> netdev name from ethX to enm<mdev_alias>, where en=Etherenet, > > >> m=mdev. > > >> >> > > > >> >> >So yes, same 12 characters are use for mdev's netdev and mdev > > >> >> >devlink port's > > >> >> phys_port_name. > > >> >> > > > >> >> >Is that what are you asking? > > >> >> > > >> >> Yes. Then you have 3 chars to handle the rest of the name (pci, > > >> >> pf)...
