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

[Parav Pandit]

> -----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?


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