Re: [PATCH v2 0/2] Simplify mtty driver and mdev core

[Jiri Pirko]

Thu, Aug 22, 2019 at 03:33:30PM CEST, parav@xxxxxxxxxxxx wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@xxxxxxxxxxx>
>> Sent: Thursday, August 22, 2019 5:50 PM
>> To: Parav Pandit <parav@xxxxxxxxxxxx>
>> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>; Jiri Pirko
>> <jiri@xxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Kirti
>> Wankhede <kwankhede@xxxxxxxxxx>; Cornelia Huck <cohuck@xxxxxxxxxx>;
>> kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; cjia <cjia@xxxxxxxxxx>;
>> netdev@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> 
>> Thu, Aug 22, 2019 at 12:04:02PM CEST, parav@xxxxxxxxxxxx wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Jiri Pirko <jiri@xxxxxxxxxxx>
>> >> Sent: Thursday, August 22, 2019 3:28 PM
>> >> To: Parav Pandit <parav@xxxxxxxxxxxx>
>> >> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>; Jiri Pirko
>> >> <jiri@xxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Kirti
>> >> Wankhede <kwankhede@xxxxxxxxxx>; Cornelia Huck
>> <cohuck@xxxxxxxxxx>;
>> >> kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; cjia
>> >> <cjia@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
>> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> >>
>> >> Thu, Aug 22, 2019 at 11:42:13AM CEST, parav@xxxxxxxxxxxx wrote:
>> >> >
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jiri Pirko <jiri@xxxxxxxxxxx>
>> >> >> Sent: Thursday, August 22, 2019 2:59 PM
>> >> >> To: Parav Pandit <parav@xxxxxxxxxxxx>
>> >> >> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>; Jiri Pirko
>> >> >> <jiri@xxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Kirti
>> >> >> Wankhede <kwankhede@xxxxxxxxxx>; Cornelia Huck
>> >> <cohuck@xxxxxxxxxx>;
>> >> >> kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; cjia
>> >> >> <cjia@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
>> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> >> >>
>> >> >> Wed, Aug 21, 2019 at 08:23:17AM CEST, parav@xxxxxxxxxxxx wrote:
>> >> >> >
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Alex Williamson <alex.williamson@xxxxxxxxxx>
>> >> >> >> Sent: Wednesday, August 21, 2019 10:56 AM
>> >> >> >> To: Parav Pandit <parav@xxxxxxxxxxxx>
>> >> >> >> Cc: Jiri Pirko <jiri@xxxxxxxxxxxx>; David S . Miller
>> >> >> >> <davem@xxxxxxxxxxxxx>; Kirti Wankhede <kwankhede@xxxxxxxxxx>;
>> >> >> >> Cornelia Huck <cohuck@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx;
>> >> >> >> linux-kernel@xxxxxxxxxxxxxxx; cjia <cjia@xxxxxxxxxx>;
>> >> >> >> netdev@xxxxxxxxxxxxxxx
>> >> >> >> 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.

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