Date:   Fri, 23 Aug 2019 10:12:21 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Parav Pandit <parav@...lanox.com>
Cc:     Alex Williamson <alex.williamson@...hat.com>,
        Jiri Pirko <jiri@...lanox.com>,
        "David S . Miller" <davem@...emloft.net>,
        Kirti Wankhede <kwankhede@...dia.com>,
        Cornelia Huck <cohuck@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        cjia <cjia@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core

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

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

Powered by blists - more mailing lists