netdev - Re: [PATCH mlx5-next v2 2/5] PCI: Add SR-IOV sysfs entry to read total number of dynamic MSI-X vectors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210116083659.GM944463@unreal>
Date:   Sat, 16 Jan 2021 10:36:59 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-pci@...r.kernel.org,
        linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
        Don Dutile <ddutile@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [PATCH mlx5-next v2 2/5] PCI: Add SR-IOV sysfs entry to read
 total number of dynamic MSI-X vectors

On Thu, Jan 14, 2021 at 05:05:36PM -0700, Alex Williamson wrote:
> On Thu, 14 Jan 2021 12:31:37 +0200
> Leon Romanovsky <leon@...nel.org> wrote:
>
> > From: Leon Romanovsky <leonro@...dia.com>
> >
> > Some SR-IOV capable devices provide an ability to configure specific
> > number of MSI-X vectors on their VF prior driver is probed on that VF.
> >
> > In order to make management easy, provide new read-only sysfs file that
> > returns a total number of possible to configure MSI-X vectors.
> >
> > cat /sys/bus/pci/devices/.../sriov_vf_total_msix
> >   = 0 - feature is not supported
> >   > 0 - total number of MSI-X vectors to consume by the VFs
> >
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-pci | 14 +++++++++++
> >  drivers/pci/iov.c                       | 31 +++++++++++++++++++++++++
> >  drivers/pci/pci.h                       |  3 +++
> >  include/linux/pci.h                     |  2 ++
> >  4 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> > index 34a8c6bcde70..530c249cc3da 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-pci
> > +++ b/Documentation/ABI/testing/sysfs-bus-pci
> > @@ -395,3 +395,17 @@ Description:
> >  		The file is writable if the PF is bound to a driver that
> >  		set sriov_vf_total_msix > 0 and there is no driver bound
> >  		to the VF.
> > +
> > +What:		/sys/bus/pci/devices/.../sriov_vf_total_msix
> > +Date:		January 2021
> > +Contact:	Leon Romanovsky <leonro@...dia.com>
> > +Description:
> > +		This file is associated with the SR-IOV PFs.
> > +		It returns a total number of possible to configure MSI-X
> > +		vectors on the enabled VFs.
> > +
> > +		The values returned are:
> > +		 * > 0 - this will be total number possible to consume by VFs,
> > +		 * = 0 - feature is not supported
>
> As with previous, why expose it if not supported?

It is much simpler to the users implement logic that operates
accordingly to this value instead of relying on exist/not-exist and
anyway handle 0 to be on the safe side.

>
> This seems pretty challenging for userspace to use; aiui they would
> need to iterate all the VFs to learn how many vectors are already
> allocated, subtract that number from this value, all while hoping they
> aren't racing someone else doing the same.  Would it be more useful if
> this reported the number of surplus vectors available?

Only privileged users are allowed to do it, so it is unlikely that we
will have more than one entity which manages PFs/VFs assignments.

Users already count number of CPUs they give to the VMs, so counting
resources is not new to them.

I didn't count in the kernel because it will require from users to
understand and treat "0" differently to understand that the pool is
depleted. So they will need to count max size of the pool anyway.

Unless we want to have two knobs, one of max and another for current,
they will count. The thing is that users will count anyway and won't
use the current value. It gives nothing.

>
> How would a per VF limit be exposed?  Do we expect users to know the
> absolutely MSI-X vector limit or the device specific limit?  Thanks,

At this stage yes, we can discuss it later when the need will arise.

Thanks