mbox series

[0/1] Use of /sys/bus/pci/devices/…/index for non-SMBIOS platforms

Message ID 20210412135905.1434249-1-schnelle@linux.ibm.com (mailing list archive)
Headers show
Series Use of /sys/bus/pci/devices/…/index for non-SMBIOS platforms | expand

Message

Niklas Schnelle April 12, 2021, 1:59 p.m. UTC
Hi Narendra, Hi All,

According to Documentation/ABI/testing/sysfs-bus-pci you are responsible
for the index device attribute that is used by systemd to create network
interface names.

Now we would like to reuse this attribute for firmware provided PCI
device index numbers on the s390 architecture which doesn't have
SMBIOS/DMI nor ACPI. All code changes are within our architecture
specific code but I'd like to get some Acks for this reuse. I've sent an
RFC version of this patch on 15th of March with the subject:

   s390/pci: expose a PCI device's UID as its index

but got no response. Would it be okay to re-use this attribute for
essentially the same purpose but with index numbers provided by
a different platform mechanism? I think this would be cleaner than
further proliferation of /sys/bus/pci/devices/<dev>/xyz_index
attributes and allows re-use of the existing userspace infrastructure.

Thanks,
Niklas Schnelle

Niklas Schnelle (1):
  s390/pci: expose a PCI device's UID as its index

 Documentation/ABI/testing/sysfs-bus-pci | 11 +++++---
 arch/s390/pci/pci_sysfs.c               | 35 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

Comments

Leon Romanovsky April 13, 2021, 5:39 a.m. UTC | #1
On Mon, Apr 12, 2021 at 03:59:04PM +0200, Niklas Schnelle wrote:
> Hi Narendra, Hi All,
> 
> According to Documentation/ABI/testing/sysfs-bus-pci you are responsible
> for the index device attribute that is used by systemd to create network
> interface names.
> 
> Now we would like to reuse this attribute for firmware provided PCI
> device index numbers on the s390 architecture which doesn't have
> SMBIOS/DMI nor ACPI. All code changes are within our architecture
> specific code but I'd like to get some Acks for this reuse. I've sent an
> RFC version of this patch on 15th of March with the subject:
> 
>    s390/pci: expose a PCI device's UID as its index
> 
> but got no response. Would it be okay to re-use this attribute for
> essentially the same purpose but with index numbers provided by
> a different platform mechanism? I think this would be cleaner than
> further proliferation of /sys/bus/pci/devices/<dev>/xyz_index
> attributes and allows re-use of the existing userspace infrastructure.

I'm missing an explanation that this change is safe for systemd and
they don't have some hard-coded assumption about the meaning of existing
index on s390.

Thanks
Niklas Schnelle April 13, 2021, 6:57 a.m. UTC | #2
On Tue, 2021-04-13 at 08:39 +0300, Leon Romanovsky wrote:
> On Mon, Apr 12, 2021 at 03:59:04PM +0200, Niklas Schnelle wrote:
> > Hi Narendra, Hi All,
> > 
> > According to Documentation/ABI/testing/sysfs-bus-pci you are responsible
> > for the index device attribute that is used by systemd to create network
> > interface names.
> > 
> > Now we would like to reuse this attribute for firmware provided PCI
> > device index numbers on the s390 architecture which doesn't have
> > SMBIOS/DMI nor ACPI. All code changes are within our architecture
> > specific code but I'd like to get some Acks for this reuse. I've sent an
> > RFC version of this patch on 15th of March with the subject:
> > 
> >    s390/pci: expose a PCI device's UID as its index
> > 
> > but got no response. Would it be okay to re-use this attribute for
> > essentially the same purpose but with index numbers provided by
> > a different platform mechanism? I think this would be cleaner than
> > further proliferation of /sys/bus/pci/devices/<dev>/xyz_index
> > attributes and allows re-use of the existing userspace infrastructure.
> 
> I'm missing an explanation that this change is safe for systemd and
> they don't have some hard-coded assumption about the meaning of existing
> index on s390.
> 
> Thanks


Sure, good point. So first off yes this change does create new index
based names also on existing systemd versions, this is known and
intended and we'll certainly closely collaborate with any distributions
wishing to backport this change.

As for being otherwise safe or having unintended consequences, Viktor
(see R-b) and I recently got the following PR merged in that exact area
of systemd to fix how hotplug slot derived interface names are
generated:
https://github.com/systemd/systemd/pull/19017
In working on that we did also analyse the use of the index attribute
for hidden assumptions and tested with this attribute added. Arguably,
as the nature of that PR shows we haven't had a perfect track record of
keeping this monitored but will in the future as PCI based NICs become
increasingly important for our platform. We also have special NIC
naming logic in the same area for our channel based platform specific
NICs which was also contributed by Viktor.

Thanks,
Niklas
Leon Romanovsky April 13, 2021, 7:32 a.m. UTC | #3
On Tue, Apr 13, 2021 at 08:57:19AM +0200, Niklas Schnelle wrote:
> On Tue, 2021-04-13 at 08:39 +0300, Leon Romanovsky wrote:
> > On Mon, Apr 12, 2021 at 03:59:04PM +0200, Niklas Schnelle wrote:
> > > Hi Narendra, Hi All,
> > > 
> > > According to Documentation/ABI/testing/sysfs-bus-pci you are responsible
> > > for the index device attribute that is used by systemd to create network
> > > interface names.
> > > 
> > > Now we would like to reuse this attribute for firmware provided PCI
> > > device index numbers on the s390 architecture which doesn't have
> > > SMBIOS/DMI nor ACPI. All code changes are within our architecture
> > > specific code but I'd like to get some Acks for this reuse. I've sent an
> > > RFC version of this patch on 15th of March with the subject:
> > > 
> > >    s390/pci: expose a PCI device's UID as its index
> > > 
> > > but got no response. Would it be okay to re-use this attribute for
> > > essentially the same purpose but with index numbers provided by
> > > a different platform mechanism? I think this would be cleaner than
> > > further proliferation of /sys/bus/pci/devices/<dev>/xyz_index
> > > attributes and allows re-use of the existing userspace infrastructure.
> > 
> > I'm missing an explanation that this change is safe for systemd and
> > they don't have some hard-coded assumption about the meaning of existing
> > index on s390.
> > 
> > Thanks
> 
> 
> Sure, good point. So first off yes this change does create new index
> based names also on existing systemd versions, this is known and
> intended and we'll certainly closely collaborate with any distributions
> wishing to backport this change.
> 
> As for being otherwise safe or having unintended consequences, Viktor
> (see R-b) and I recently got the following PR merged in that exact area
> of systemd to fix how hotplug slot derived interface names are
> generated:
> https://github.com/systemd/systemd/pull/19017
> In working on that we did also analyse the use of the index attribute
> for hidden assumptions and tested with this attribute added. Arguably,
> as the nature of that PR shows we haven't had a perfect track record of
> keeping this monitored but will in the future as PCI based NICs become
> increasingly important for our platform. We also have special NIC
> naming logic in the same area for our channel based platform specific
> NICs which was also contributed by Viktor.

Thanks, this PR is exciting to read, very warm words were said about
kernel developers :). Can you please summarize that will be the breakage
in old systemd if this index will be overloaded?

Thanks

> 
> Thanks,
> Niklas
>
Niklas Schnelle April 13, 2021, 7:53 a.m. UTC | #4
On Tue, 2021-04-13 at 10:32 +0300, Leon Romanovsky wrote:
> On Tue, Apr 13, 2021 at 08:57:19AM +0200, Niklas Schnelle wrote:
> > On Tue, 2021-04-13 at 08:39 +0300, Leon Romanovsky wrote:
> > > On Mon, Apr 12, 2021 at 03:59:04PM +0200, Niklas Schnelle wrote:
> > > > Hi Narendra, Hi All,
> > > > 
> > > > According to Documentation/ABI/testing/sysfs-bus-pci you are responsible
> > > > for the index device attribute that is used by systemd to create network
> > > > interface names.
> > > > 
> > > > Now we would like to reuse this attribute for firmware provided PCI
> > > > device index numbers on the s390 architecture which doesn't have
> > > > SMBIOS/DMI nor ACPI. All code changes are within our architecture
> > > > specific code but I'd like to get some Acks for this reuse. I've sent an
> > > > RFC version of this patch on 15th of March with the subject:
> > > > 
> > > >    s390/pci: expose a PCI device's UID as its index
> > > > 
> > > > but got no response. Would it be okay to re-use this attribute for
> > > > essentially the same purpose but with index numbers provided by
> > > > a different platform mechanism? I think this would be cleaner than
> > > > further proliferation of /sys/bus/pci/devices/<dev>/xyz_index
> > > > attributes and allows re-use of the existing userspace infrastructure.
> > > 
> > > I'm missing an explanation that this change is safe for systemd and
> > > they don't have some hard-coded assumption about the meaning of existing
> > > index on s390.
> > > 
> > > Thanks
> > 
> > Sure, good point. So first off yes this change does create new index
> > based names also on existing systemd versions, this is known and
> > intended and we'll certainly closely collaborate with any distributions
> > wishing to backport this change.
> > 
> > As for being otherwise safe or having unintended consequences, Viktor
> > (see R-b) and I recently got the following PR merged in that exact area
> > of systemd to fix how hotplug slot derived interface names are
> > generated:
> > https://github.com/systemd/systemd/pull/19017
> > In working on that we did also analyse the use of the index attribute
> > for hidden assumptions and tested with this attribute added. Arguably,
> > as the nature of that PR shows we haven't had a perfect track record of
> > keeping this monitored but will in the future as PCI based NICs become
> > increasingly important for our platform. We also have special NIC
> > naming logic in the same area for our channel based platform specific
> > NICs which was also contributed by Viktor.
> 
> Thanks, this PR is exciting to read, very warm words were said about
> kernel developers :). Can you please summarize that will be the breakage
> in old systemd if this index will be overloaded?
> 
> Thanks

;-) yeah, maybe us working closely across kernel and systemd will help
improve relations.

In principle the same happens on existing and new systemd i.e. we get
new eno<UID_in_decimal>… interface names. Due to the way naming
priorities work this will become the dominant interface name but thanks
to altname support in current kernels we expect all old settings,
routing rules etc. to remain working with the old name.

This only happens however if the Linux instance is running with active
UID Uniqueness Checking, this is a Hypervisor/Platform setting that
enforces that no PCI device can be attached if one with the same UID is
already attached. So far this setting was hidden inside the kernel but
I recently committed a change to expose it to userspace here:
https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=408f2c9c15682fc21b645fdec1f726492e235c4b

That said, we're also still open for other approaches for example as
floated by Lennart in this comment:
https://github.com/systemd/systemd/pull/18829#discussion_r584794766
But this of course also depends on whether the kernel community is open
to reusing the index attribute or has other preferences.

> 
> > Thanks,
> > Niklas
> >
K, Narendra April 13, 2021, 6:22 p.m. UTC | #5
> -----Original Message-----
> From: Niklas Schnelle <schnelle@linux.ibm.com>
> Sent: Monday, April 12, 2021 7:29 PM
> To: K, Narendra
> Cc: Viktor Mihajlovski; Stefan Raspl; Peter Oberparleiter; linux-
> netdev@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-s390@vger.kernel.org
> Subject: [PATCH 0/1] Use of /sys/bus/pci/devices/…/index for non-SMBIOS
> platforms
> 
> 
> [EXTERNAL EMAIL]
> 
> Hi Narendra, Hi All,
> 
> According to Documentation/ABI/testing/sysfs-bus-pci you are responsible for
> the index device attribute that is used by systemd to create network interface
> names.
> 
> Now we would like to reuse this attribute for firmware provided PCI device
> index numbers on the s390 architecture which doesn't have SMBIOS/DMI nor
> ACPI. All code changes are within our architecture specific code but I'd like to
> get some Acks for this reuse. I've sent an RFC version of this patch on 15th of
> March with the subject:
> 
>    s390/pci: expose a PCI device's UID as its index
> 
> but got no response. 

Hi Niklas,

Sorry, I missed responding to the RFC sent earlier.  

> Would it be okay to re-use this attribute for essentially
> the same purpose but with index numbers provided by a different platform
> mechanism? 

I will go through the request above and provide comments in two or three days.
 
With regards,
Narendra K