From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 88B3A1B710 for ; Wed, 9 May 2018 19:05:13 +0200 (CEST) Received: by mail-wm0-f68.google.com with SMTP id a8so26074900wmg.5 for ; Wed, 09 May 2018 10:05:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=59Am9DyMwKxLs3N4REmL86vkXh4vJE/QqRY6RF9o3dw=; b=jnkywhNdNp/zfLJwxQiYNnrPTM7uq0lY2/a8JH3wwwmFSGcgPS3GrxoK6fC9/M1WbQ M1nVJZovTPpHccMRjgsCCuO3lJu32MgUOeZaVbaskbmjR2/NzKt7F8z7aRs+ar18Nr6c DByGJ2dVlwaBKm0lURYw9bDBwpkLr3N35FY4+fIzYrW4tghr5raOlHNllpcMAUz4sl/1 blU7g9KfRI2eeqijYuxkeTeo1stfmUW8xE9ZlEtqh1M3U3qy8QB+kMpyiQzwcrsfpFya wE0FMpiFi0dweH7EeFdQZsqGFBJJNm963aMI/zYXhg75esEmBpV3y0Y64JiUyZHN4zGz o1DQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=59Am9DyMwKxLs3N4REmL86vkXh4vJE/QqRY6RF9o3dw=; b=gNTBRv2Y25AOuNno8fwf1K8fQW2svGIDyHlDCcjfVZyqrl04MysKBTceLmAJ3OaSic JttXxUdJ2dfyaPcXxJ0DvGBdEudlkxDUsJ/2gepRCRhh66sfY+Nm5J7Peb+F/3zka0mz iOQBtnLmRbqwh1XDdXpsyMjzhaE5zVcpLpVBz9VCuQkZhV1tAqO6qiGVSAX0yH+sV/85 f3O/WaADD6/AsEEUfSLASO4WH7IhkEZfiR0ldhZy3MRU+HETjmlsZd9xO4XxD1HMhztp TnfJVtf65+MMXu6ECMZNS/7iUKhRqC90ijDubvKMbcfbk1T9kFjFLwoQRPStoVBv84SN IELA== X-Gm-Message-State: ALQs6tBeQyPnlLax00zQXb7rB7DSJZRILP1WndCKcAvF7pImaiL/vK5/ MiN/UdHmxTCdcFvzPL1k8tPJJxoGI6yfdnUwpLgeTw== X-Google-Smtp-Source: AB8JxZo5Vt0/03NZB9UhZYSBSs7fKIRNiDZGCH+e2MjQcekLPhyGvHffGQJVIdgt0SiLyb8P+a10jlMRj7mPsPOprnQ= X-Received: by 2002:a50:a621:: with SMTP id d30-v6mr60848977edc.173.1525885512996; Wed, 09 May 2018 10:05:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.212.155 with HTTP; Wed, 9 May 2018 10:05:12 -0700 (PDT) In-Reply-To: <20180430172040.13506-3-aconole@redhat.com> References: <20180430172040.13506-1-aconole@redhat.com> <20180430172040.13506-3-aconole@redhat.com> From: Alejandro Lucero Date: Wed, 9 May 2018 18:05:12 +0100 Message-ID: To: Aaron Conole Cc: stable@dpdk.org, Ferruh Yigit , Yuanhan Liu , Eelco Chaudron , Pablo Cascon , Kevin Traynor , Adrien Mazarguil Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 May 2018 17:05:13 -0000 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. 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. 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. On Mon, Apr 30, 2018 at 6:20 PM, Aaron Conole 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 > Signed-off-by: Aaron Conole > --- > 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 > >