[dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
Alejandro Lucero
alejandro.lucero at netronome.com
Wed May 9 21:44:06 CEST 2018
On Wed, May 9, 2018 at 6:53 PM, Aaron Conole <aconole at redhat.com> wrote:
> Alejandro Lucero <alejandro.lucero at netronome.com> writes:
>
> > I have been thinking about this, and there was something that did not
> seem right to me, although I
> > could not explain what exactly. But this was because we have been
> thinking about VFIO and we have
> > forgotten UIO. The point is, the lock is not required with VFIO but it
> is with UIO, and I'm afraid the
> > way we are trying to solve the non-root user problem is not the right
> one.
>
> Okay, makes sense.
>
> > With VFIO the BARs mapping is done through the kernel VFIO driver, so
> once the device is bound to
> > the driver, and someone tries to use that device, the VFIO driver
> ensures there will not be another
> > user trying to access the device. However, with UIO the driver is not
> doing the BAR mapping but it is
> > the app using the sysfs resource files for that device. It could be, and
> in fact it is easy to happen, two
> > DPDK apps using the same device, because with DPDK apps there is no
> awareness of what other
> > DPDK apps are doing. It is in this case where the lock is required, and
> creating the lock file in the
> > user's home directory is not going to help.
>
> Also makes sense.
>
> > I know RH is just interested in using VFIO, but there are other
> potential NFP PMD users who may
> > want to use UIO instead, and the current solution does not offer the
> right protection in that case.
> > Maybe the lock patch could be created based on the driver the device is
> bound to, leaving /var/lock
> > for UIO and the suggested path for the VFIO case. Or even no lock at all
> for the VFIO case.
>
> Well, yes and no. For instance, in the uio case, they will be running
> most likely as the root user (because to run as non-root in the uio case
> would cause other problems). And in that case, $HOME for all
> applications will be root, yes? I think then, it won't matter.
>
>
I think "most likely" is not enough. If RH wants to run OVS as non-root,
don't you think there could be other people with same necessities? and
using UIO because IOMMU is not available?
> I do agree with no lock for the vfio case would be best. Is it relevant
> for the newer NFP driver? I haven't had a look yet. If so, then I
> think your proposals are fine there.
>
> For the older one, I only know of one patch that needs to go in (which
> fixes the accidentally lost unlink() call). After that, I wouldn't
> expect more changes in this area (since each one risks stability of the
> code base for a driver that has been replaced).
>
> > On Mon, Apr 30, 2018 at 6:20 PM, Aaron Conole <aconole at redhat.com>
> wrote:
> >
> > Currently, the nfp lock files are taken from the global lock file
> > location, which will work when the user is running as root. However,
> > some distributions and applications (notably ovs 2.8+ on RHEL/Fedora)
> > run as a non-root user.
> >
> > Acked-by: Alejandro Lucero <alejandro.lucero at netronome.com>
> > Signed-off-by: Aaron Conole <aconole at redhat.com>
> > ---
> > drivers/net/nfp/nfp_nfpu.c | 23 ++++++++++++++++++-----
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/nfp/nfp_nfpu.c b/drivers/net/nfp/nfp_nfpu.c
> > index 2ed985ff4..ae2e07220 100644
> > --- a/drivers/net/nfp/nfp_nfpu.c
> > +++ b/drivers/net/nfp/nfp_nfpu.c
> > @@ -18,6 +18,22 @@
> > #define NFP_CFG_EXP_BAR 7
> >
> > #define NFP_CFG_EXP_BAR_CFG_BASE 0x30000
> > +#define NFP_LOCKFILE_PATH_FMT "%s/nfp%d"
> > +
> > +/* get nfp lock file path (/var/lock if root, $HOME otherwise) */
> > +static void
> > +nspu_get_lockfile_path(char *buffer, int bufsz, nfpu_desc_t *desc)
> > +{
> > + const char *dir = "/var/lock";
> > + const char *home_dir = getenv("HOME");
> > +
> > + if (getuid() != 0 && home_dir != NULL)
> > + dir = home_dir;
> > +
> > + /* use current prefix as file path */
> > + snprintf(buffer, bufsz, NFP_LOCKFILE_PATH_FMT, dir,
> > + desc->nfp);
> > +}
> >
> > /* There could be other NFP userspace tools using the NSP interface.
> > * Make sure there is no other process using it and locking the access
> for
> > @@ -30,9 +46,7 @@ nspv_aquire_process_lock(nfpu_desc_t *desc)
> > struct flock lock;
> > char lockname[30];
> >
> > - memset(&lock, 0, sizeof(lock));
> > -
> > - snprintf(lockname, sizeof(lockname), "/var/lock/nfp%d",
> desc->nfp);
> > + nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
> >
> > /* Using S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> S_IWOTH */
> > desc->lock = open(lockname, O_RDWR | O_CREAT, 0666);
> > @@ -106,7 +120,6 @@ nfpu_close(nfpu_desc_t *desc)
> > rte_free(desc->nspu);
> > close(desc->lock);
> >
> > - snprintf(lockname, sizeof(lockname), "/var/lock/nfp%d",
> desc->nfp);
> > - unlink(lockname);
> > + nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
> > return 0;
> > }
> > --
> > 2.14.3
>
More information about the stable
mailing list