patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
@ 2018-04-30 17:20 Aaron Conole
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 1/2] nfp: unlink the appropriate lock file Aaron Conole
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Aaron Conole @ 2018-04-30 17:20 UTC (permalink / raw)
  To: stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

Even when trying to use the Netronome cards via VFIO, there is a
lock file being requested, which requires root access.  This series
fixes an issue with the nfp driver lock release always releasing
'nfp0' (which isn't a bug in practice for other reasons) as well
as allowing the lock file location to be configured by the application
or user via the HOME environment variable (similar to other resources
in the DPDK framework).

This series is only applicable to the stable tree because the nfp
driver is completely rewritten for later versions of DPDK.

Aaron Conole (2):
  nfp: unlink the appropriate lock file
  nfp: allow for non-root user

 drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-stable] [PATCH 1/2] nfp: unlink the appropriate lock file
  2018-04-30 17:20 [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Aaron Conole
@ 2018-04-30 17:20 ` Aaron Conole
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user Aaron Conole
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Aaron Conole @ 2018-04-30 17:20 UTC (permalink / raw)
  To: stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

The nfpu_close needs to unlink the lock file associated with the
nfp descriptor, not lock file 0.

Fixes: d12206e00590 ("net/nfp: add NSP user space interface")

Cc: stable@dpdk.org
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 drivers/net/nfp/nfp_nfpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_nfpu.c b/drivers/net/nfp/nfp_nfpu.c
index f11afef35..2ed985ff4 100644
--- a/drivers/net/nfp/nfp_nfpu.c
+++ b/drivers/net/nfp/nfp_nfpu.c
@@ -101,8 +101,12 @@ nfpu_open(struct rte_pci_device *pci_dev, nfpu_desc_t *desc, int nfp)
 int
 nfpu_close(nfpu_desc_t *desc)
 {
+	char lockname[30];
+
 	rte_free(desc->nspu);
 	close(desc->lock);
-	unlink("/var/lock/nfp0");
+
+	snprintf(lockname, sizeof(lockname), "/var/lock/nfp%d", desc->nfp);
+	unlink(lockname);
 	return 0;
 }
-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-04-30 17:20 [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Aaron Conole
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 1/2] nfp: unlink the appropriate lock file Aaron Conole
@ 2018-04-30 17:20 ` Aaron Conole
  2018-05-08 13:09   ` Eelco Chaudron
  2018-05-09 17:05   ` Alejandro Lucero
  2018-04-30 18:02 ` [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Kevin Traynor
  2018-05-01 10:36 ` Luca Boccassi
  3 siblings, 2 replies; 19+ messages in thread
From: Aaron Conole @ 2018-04-30 17:20 UTC (permalink / raw)
  To: stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

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@netronome.com>
Signed-off-by: Aaron Conole <aconole@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-04-30 17:20 [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Aaron Conole
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 1/2] nfp: unlink the appropriate lock file Aaron Conole
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user Aaron Conole
@ 2018-04-30 18:02 ` Kevin Traynor
  2018-05-06  6:34   ` Yuanhan Liu
  2018-05-01 10:36 ` Luca Boccassi
  3 siblings, 1 reply; 19+ messages in thread
From: Kevin Traynor @ 2018-04-30 18:02 UTC (permalink / raw)
  To: Aaron Conole, stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Adrien Mazarguil

On 04/30/2018 06:20 PM, Aaron Conole wrote:
> Even when trying to use the Netronome cards via VFIO, there is a
> lock file being requested, which requires root access.  This series
> fixes an issue with the nfp driver lock release always releasing
> 'nfp0' (which isn't a bug in practice for other reasons) as well
> as allowing the lock file location to be configured by the application
> or user via the HOME environment variable (similar to other resources
> in the DPDK framework).
> 
> This series is only applicable to the stable tree because the nfp
> driver is completely rewritten for later versions of DPDK.
> 

Hi Yuanhan, I think we would like to get this into the next 17.11 stable
release if possible. Not sure when you are merging patches into the tree
for it?

Kevin.

> Aaron Conole (2):
>   nfp: unlink the appropriate lock file
>   nfp: allow for non-root user
> 
>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-04-30 17:20 [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Aaron Conole
                   ` (2 preceding siblings ...)
  2018-04-30 18:02 ` [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Kevin Traynor
@ 2018-05-01 10:36 ` Luca Boccassi
  2018-05-03 10:30   ` Luca Boccassi
  3 siblings, 1 reply; 19+ messages in thread
From: Luca Boccassi @ 2018-05-01 10:36 UTC (permalink / raw)
  To: Aaron Conole, stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

On Mon, 2018-04-30 at 13:20 -0400, Aaron Conole wrote:
> Even when trying to use the Netronome cards via VFIO, there is a
> lock file being requested, which requires root access.  This series
> fixes an issue with the nfp driver lock release always releasing
> 'nfp0' (which isn't a bug in practice for other reasons) as well
> as allowing the lock file location to be configured by the
> application
> or user via the HOME environment variable (similar to other resources
> in the DPDK framework).
> 
> This series is only applicable to the stable tree because the nfp
> driver is completely rewritten for later versions of DPDK.
> 
> Aaron Conole (2):
>   nfp: unlink the appropriate lock file
>   nfp: allow for non-root user
> 
>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)

Series applied to 18.02, thanks.

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-01 10:36 ` Luca Boccassi
@ 2018-05-03 10:30   ` Luca Boccassi
  2018-05-03 12:25     ` Alejandro Lucero
  0 siblings, 1 reply; 19+ messages in thread
From: Luca Boccassi @ 2018-05-03 10:30 UTC (permalink / raw)
  To: Aaron Conole, stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

On Tue, 2018-05-01 at 11:36 +0100, Luca Boccassi wrote:
> On Mon, 2018-04-30 at 13:20 -0400, Aaron Conole wrote:
> > Even when trying to use the Netronome cards via VFIO, there is a
> > lock file being requested, which requires root access.  This series
> > fixes an issue with the nfp driver lock release always releasing
> > 'nfp0' (which isn't a bug in practice for other reasons) as well
> > as allowing the lock file location to be configured by the
> > application
> > or user via the HOME environment variable (similar to other
> > resources
> > in the DPDK framework).
> > 
> > This series is only applicable to the stable tree because the nfp
> > driver is completely rewritten for later versions of DPDK.
> > 
> > Aaron Conole (2):
> >   nfp: unlink the appropriate lock file
> >   nfp: allow for non-root user
> > 
> >  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> Series applied to 18.02, thanks.

I just noticed this is not in dpdk/master yet, is it scheduled for a
later release?

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-03 10:30   ` Luca Boccassi
@ 2018-05-03 12:25     ` Alejandro Lucero
  2018-05-03 12:57       ` Luca Boccassi
  0 siblings, 1 reply; 19+ messages in thread
From: Alejandro Lucero @ 2018-05-03 12:25 UTC (permalink / raw)
  To: Luca Boccassi
  Cc: Aaron Conole, stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

This fixes can not be applied to master because that code was changed and
it is not there anymore.

So it these fixes are going to be applied just to stable versions.

On Thu, May 3, 2018 at 11:30 AM, Luca Boccassi <bluca@debian.org> wrote:

> On Tue, 2018-05-01 at 11:36 +0100, Luca Boccassi wrote:
> > On Mon, 2018-04-30 at 13:20 -0400, Aaron Conole wrote:
> > > Even when trying to use the Netronome cards via VFIO, there is a
> > > lock file being requested, which requires root access.  This series
> > > fixes an issue with the nfp driver lock release always releasing
> > > 'nfp0' (which isn't a bug in practice for other reasons) as well
> > > as allowing the lock file location to be configured by the
> > > application
> > > or user via the HOME environment variable (similar to other
> > > resources
> > > in the DPDK framework).
> > >
> > > This series is only applicable to the stable tree because the nfp
> > > driver is completely rewritten for later versions of DPDK.
> > >
> > > Aaron Conole (2):
> > >   nfp: unlink the appropriate lock file
> > >   nfp: allow for non-root user
> > >
> > >  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
> > >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > Series applied to 18.02, thanks.
>
> I just noticed this is not in dpdk/master yet, is it scheduled for a
> later release?
>
> --
> Kind regards,
> Luca Boccassi
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-03 12:25     ` Alejandro Lucero
@ 2018-05-03 12:57       ` Luca Boccassi
  0 siblings, 0 replies; 19+ messages in thread
From: Luca Boccassi @ 2018-05-03 12:57 UTC (permalink / raw)
  To: Alejandro Lucero
  Cc: Aaron Conole, stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

On Thu, 2018-05-03 at 13:25 +0100, Alejandro Lucero wrote:
> This fixes can not be applied to master because that code was changed
> and
> it is not there anymore.
> 
> So it these fixes are going to be applied just to stable versions.

Ok, makese sense, thanks. Will push to 18.02.2 with the next batch.

> On Thu, May 3, 2018 at 11:30 AM, Luca Boccassi <bluca@debian.org>
> wrote:
> 
> > On Tue, 2018-05-01 at 11:36 +0100, Luca Boccassi wrote:
> > > On Mon, 2018-04-30 at 13:20 -0400, Aaron Conole wrote:
> > > > Even when trying to use the Netronome cards via VFIO, there is
> > > > a
> > > > lock file being requested, which requires root access.  This
> > > > series
> > > > fixes an issue with the nfp driver lock release always
> > > > releasing
> > > > 'nfp0' (which isn't a bug in practice for other reasons) as
> > > > well
> > > > as allowing the lock file location to be configured by the
> > > > application
> > > > or user via the HOME environment variable (similar to other
> > > > resources
> > > > in the DPDK framework).
> > > > 
> > > > This series is only applicable to the stable tree because the
> > > > nfp
> > > > driver is completely rewritten for later versions of DPDK.
> > > > 
> > > > Aaron Conole (2):
> > > >   nfp: unlink the appropriate lock file
> > > >   nfp: allow for non-root user
> > > > 
> > > >  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
> > > >  1 file changed, 21 insertions(+), 4 deletions(-)
> > > 
> > > Series applied to 18.02, thanks.
> > 
> > I just noticed this is not in dpdk/master yet, is it scheduled for
> > a
> > later release?
> > 
> > --
> > Kind regards,
> > Luca Boccassi
> > 

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-04-30 18:02 ` [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Kevin Traynor
@ 2018-05-06  6:34   ` Yuanhan Liu
  2018-05-08  9:23     ` Kevin Traynor
  0 siblings, 1 reply; 19+ messages in thread
From: Yuanhan Liu @ 2018-05-06  6:34 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: Aaron Conole, stable, Alejandro Lucero, Ferruh Yigit,
	Eelco Chaudron, Pablo Cascon, Adrien Mazarguil

On Mon, Apr 30, 2018 at 07:02:29PM +0100, Kevin Traynor wrote:
> On 04/30/2018 06:20 PM, Aaron Conole wrote:
> > Even when trying to use the Netronome cards via VFIO, there is a
> > lock file being requested, which requires root access.  This series
> > fixes an issue with the nfp driver lock release always releasing
> > 'nfp0' (which isn't a bug in practice for other reasons) as well
> > as allowing the lock file location to be configured by the application
> > or user via the HOME environment variable (similar to other resources
> > in the DPDK framework).
> > 
> > This series is only applicable to the stable tree because the nfp
> > driver is completely rewritten for later versions of DPDK.
> > 
> 
> Hi Yuanhan, I think we would like to get this into the next 17.11 stable
> release if possible. Not sure when you are merging patches into the tree
> for it?

Hi,

I have just applied it to dpdk-stable/17.11. It will be part of 17.11.3
LTS release.

Thanks.

	--yliu
> 
> Kevin.
> 
> > Aaron Conole (2):
> >   nfp: unlink the appropriate lock file
> >   nfp: allow for non-root user
> > 
> >  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> > 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-06  6:34   ` Yuanhan Liu
@ 2018-05-08  9:23     ` Kevin Traynor
  2018-05-14 14:23       ` Kevin Traynor
  0 siblings, 1 reply; 19+ messages in thread
From: Kevin Traynor @ 2018-05-08  9:23 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: Aaron Conole, stable, Alejandro Lucero, Ferruh Yigit,
	Eelco Chaudron, Pablo Cascon, Adrien Mazarguil

On 05/06/2018 07:34 AM, Yuanhan Liu wrote:
> On Mon, Apr 30, 2018 at 07:02:29PM +0100, Kevin Traynor wrote:
>> On 04/30/2018 06:20 PM, Aaron Conole wrote:
>>> Even when trying to use the Netronome cards via VFIO, there is a
>>> lock file being requested, which requires root access.  This series
>>> fixes an issue with the nfp driver lock release always releasing
>>> 'nfp0' (which isn't a bug in practice for other reasons) as well
>>> as allowing the lock file location to be configured by the application
>>> or user via the HOME environment variable (similar to other resources
>>> in the DPDK framework).
>>>
>>> This series is only applicable to the stable tree because the nfp
>>> driver is completely rewritten for later versions of DPDK.
>>>
>>
>> Hi Yuanhan, I think we would like to get this into the next 17.11 stable
>> release if possible. Not sure when you are merging patches into the tree
>> for it?
> 
> Hi,
> 
> I have just applied it to dpdk-stable/17.11. It will be part of 17.11.3
> LTS release.
> 

Hi,

They don't appear to be on the branch? Maybe you have a private 'next'
branch or something in your workflow, but reporting in case something
still needs to be pushed.

thanks,
Kevin.

> Thanks.
> 
> 	--yliu
>>
>> Kevin.
>>
>>> Aaron Conole (2):
>>>   nfp: unlink the appropriate lock file
>>>   nfp: allow for non-root user
>>>
>>>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user Aaron Conole
@ 2018-05-08 13:09   ` Eelco Chaudron
  2018-05-08 13:20     ` Aaron Conole
  2018-05-09 17:05   ` Alejandro Lucero
  1 sibling, 1 reply; 19+ messages in thread
From: Eelco Chaudron @ 2018-05-08 13:09 UTC (permalink / raw)
  To: Aaron Conole, stable
  Cc: Alejandro Lucero, Ferruh Yigit, Yuanhan Liu, Pablo Cascon,
	Kevin Traynor, Adrien Mazarguil

On 30/04/18 19:20, 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 <alejandro.lucero@netronome.com>
> Signed-off-by: Aaron Conole <aconole@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);
Sorry for being late, but was this unlink() removed by accident? And 
should be below nspu_get_lockfile_path()
> +	nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
>   	return 0;
>   }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-05-08 13:09   ` Eelco Chaudron
@ 2018-05-08 13:20     ` Aaron Conole
  0 siblings, 0 replies; 19+ messages in thread
From: Aaron Conole @ 2018-05-08 13:20 UTC (permalink / raw)
  To: Eelco Chaudron
  Cc: stable, Alejandro Lucero, Ferruh Yigit, Yuanhan Liu,
	Pablo Cascon, Kevin Traynor, Adrien Mazarguil

Eelco Chaudron <echaudro@redhat.com> writes:

> On 30/04/18 19:20, 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 <alejandro.lucero@netronome.com>
>> Signed-off-by: Aaron Conole <aconole@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);
> Sorry for being late, but was this unlink() removed by accident? And
> should be below nspu_get_lockfile_path()

That looks like a bug.  If you have a patch already to go, please submit
it.  If not, I can cook one up.

Thanks for the catch, Eelco!

>> +	nspu_get_lockfile_path(lockname, sizeof(lockname), desc);
>>   	return 0;
>>   }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-04-30 17:20 ` [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user Aaron Conole
  2018-05-08 13:09   ` Eelco Chaudron
@ 2018-05-09 17:05   ` Alejandro Lucero
  2018-05-09 17:53     ` Aaron Conole
  1 sibling, 1 reply; 19+ messages in thread
From: Alejandro Lucero @ 2018-05-09 17:05 UTC (permalink / raw)
  To: Aaron Conole
  Cc: stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron, Pablo Cascon,
	Kevin Traynor, Adrien Mazarguil

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 <aconole@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@netronome.com>
> Signed-off-by: Aaron Conole <aconole@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
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-05-09 17:05   ` Alejandro Lucero
@ 2018-05-09 17:53     ` Aaron Conole
  2018-05-09 19:44       ` Alejandro Lucero
  0 siblings, 1 reply; 19+ messages in thread
From: Aaron Conole @ 2018-05-09 17:53 UTC (permalink / raw)
  To: Alejandro Lucero
  Cc: stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron, Pablo Cascon,
	Kevin Traynor, Adrien Mazarguil

Alejandro Lucero <alejandro.lucero@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 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@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@netronome.com>
>  Signed-off-by: Aaron Conole <aconole@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-05-09 17:53     ` Aaron Conole
@ 2018-05-09 19:44       ` Alejandro Lucero
  2018-05-10 12:00         ` Alejandro Lucero
  0 siblings, 1 reply; 19+ messages in thread
From: Alejandro Lucero @ 2018-05-09 19:44 UTC (permalink / raw)
  To: Aaron Conole
  Cc: stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron, Pablo Cascon,
	Kevin Traynor, Adrien Mazarguil

On Wed, May 9, 2018 at 6:53 PM, Aaron Conole <aconole@redhat.com> wrote:

> Alejandro Lucero <alejandro.lucero@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@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@netronome.com>
> >  Signed-off-by: Aaron Conole <aconole@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
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user
  2018-05-09 19:44       ` Alejandro Lucero
@ 2018-05-10 12:00         ` Alejandro Lucero
  0 siblings, 0 replies; 19+ messages in thread
From: Alejandro Lucero @ 2018-05-10 12:00 UTC (permalink / raw)
  To: Aaron Conole
  Cc: stable, Ferruh Yigit, Yuanhan Liu, Eelco Chaudron, Pablo Cascon,
	Kevin Traynor, Adrien Mazarguil

On Wed, May 9, 2018 at 8:44 PM, Alejandro Lucero <
alejandro.lucero@netronome.com> wrote:

>
>
> On Wed, May 9, 2018 at 6:53 PM, Aaron Conole <aconole@redhat.com> wrote:
>
>> Alejandro Lucero <alejandro.lucero@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 have been doing some tests and it turns out it is not possible to run
DPDK apps as non-root user. There are several files that need to be
accessed and it does require to give permissions beforehand to that
non-root user, which can be done. But it is not possible for non-root users
to get the hugepages physical addresses, because the pagemap file at
/proc/self/ is just accessible to root and this file is created by the
kernel when the process is created. For VFIO, the hugepages physical
address is not required by the PMD but just IOVAs, and the VFIO driver will
create the IOMMU mappings with those IOVAs and the right physical addresses.

Therefore, I think it is safe enough to keep the user home directory for
the lock file, which for the UIO case will always be the root directory.


> 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@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@netronome.com>
>> >  Signed-off-by: Aaron Conole <aconole@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
>>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-08  9:23     ` Kevin Traynor
@ 2018-05-14 14:23       ` Kevin Traynor
  2018-05-20  7:19         ` Yuanhan Liu
  0 siblings, 1 reply; 19+ messages in thread
From: Kevin Traynor @ 2018-05-14 14:23 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: Aaron Conole, stable, Alejandro Lucero, Ferruh Yigit,
	Eelco Chaudron, Pablo Cascon, Adrien Mazarguil, Thomas Monjalon

Ping Yuanhan - can you clarify question below? Also, can you let us know
when you are planning further backports to 17.11 stable branch?

thanks,
Kevin.

On 05/08/2018 10:23 AM, Kevin Traynor wrote:
> On 05/06/2018 07:34 AM, Yuanhan Liu wrote:
>> On Mon, Apr 30, 2018 at 07:02:29PM +0100, Kevin Traynor wrote:
>>> On 04/30/2018 06:20 PM, Aaron Conole wrote:
>>>> Even when trying to use the Netronome cards via VFIO, there is a
>>>> lock file being requested, which requires root access.  This series
>>>> fixes an issue with the nfp driver lock release always releasing
>>>> 'nfp0' (which isn't a bug in practice for other reasons) as well
>>>> as allowing the lock file location to be configured by the application
>>>> or user via the HOME environment variable (similar to other resources
>>>> in the DPDK framework).
>>>>
>>>> This series is only applicable to the stable tree because the nfp
>>>> driver is completely rewritten for later versions of DPDK.
>>>>
>>>
>>> Hi Yuanhan, I think we would like to get this into the next 17.11 stable
>>> release if possible. Not sure when you are merging patches into the tree
>>> for it?
>>
>> Hi,
>>
>> I have just applied it to dpdk-stable/17.11. It will be part of 17.11.3
>> LTS release.
>>
> 
> Hi,
> 
> They don't appear to be on the branch? Maybe you have a private 'next'
> branch or something in your workflow, but reporting in case something
> still needs to be pushed.
> 
> thanks,
> Kevin.
> 
>> Thanks.
>>
>> 	--yliu
>>>
>>> Kevin.
>>>
>>>> Aaron Conole (2):
>>>>   nfp: unlink the appropriate lock file
>>>>   nfp: allow for non-root user
>>>>
>>>>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
>>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-14 14:23       ` Kevin Traynor
@ 2018-05-20  7:19         ` Yuanhan Liu
  2018-05-22 10:45           ` Kevin Traynor
  0 siblings, 1 reply; 19+ messages in thread
From: Yuanhan Liu @ 2018-05-20  7:19 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: Aaron Conole, stable, Alejandro Lucero, Ferruh Yigit,
	Eelco Chaudron, Pablo Cascon, Adrien Mazarguil, Thomas Monjalon

On Mon, May 14, 2018 at 03:23:41PM +0100, Kevin Traynor wrote:
> Ping Yuanhan - can you clarify question below? Also, can you let us know

Hi Kevin,

Sorry for being late: I just have very little time at weekends. I planned
to handle it last week, but I had to do something else.

And to your question: yes, they are not pushed yet: I should have done that
weeks ago though.

I will push them today.

> when you are planning further backports to 17.11 stable branch?

I just want to know do you expect something else to be released in 17.11.3?
If so, could you do a list, just for making sure I don't miss it.

	--yliu
> 
> thanks,
> Kevin.
> 
> On 05/08/2018 10:23 AM, Kevin Traynor wrote:
> > On 05/06/2018 07:34 AM, Yuanhan Liu wrote:
> >> On Mon, Apr 30, 2018 at 07:02:29PM +0100, Kevin Traynor wrote:
> >>> On 04/30/2018 06:20 PM, Aaron Conole wrote:
> >>>> Even when trying to use the Netronome cards via VFIO, there is a
> >>>> lock file being requested, which requires root access.  This series
> >>>> fixes an issue with the nfp driver lock release always releasing
> >>>> 'nfp0' (which isn't a bug in practice for other reasons) as well
> >>>> as allowing the lock file location to be configured by the application
> >>>> or user via the HOME environment variable (similar to other resources
> >>>> in the DPDK framework).
> >>>>
> >>>> This series is only applicable to the stable tree because the nfp
> >>>> driver is completely rewritten for later versions of DPDK.
> >>>>
> >>>
> >>> Hi Yuanhan, I think we would like to get this into the next 17.11 stable
> >>> release if possible. Not sure when you are merging patches into the tree
> >>> for it?
> >>
> >> Hi,
> >>
> >> I have just applied it to dpdk-stable/17.11. It will be part of 17.11.3
> >> LTS release.
> >>
> > 
> > Hi,
> > 
> > They don't appear to be on the branch? Maybe you have a private 'next'
> > branch or something in your workflow, but reporting in case something
> > still needs to be pushed.
> > 
> > thanks,
> > Kevin.
> > 
> >> Thanks.
> >>
> >> 	--yliu
> >>>
> >>> Kevin.
> >>>
> >>>> Aaron Conole (2):
> >>>>   nfp: unlink the appropriate lock file
> >>>>   nfp: allow for non-root user
> >>>>
> >>>>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
> >>>>  1 file changed, 21 insertions(+), 4 deletions(-)
> >>>>
> > 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome
  2018-05-20  7:19         ` Yuanhan Liu
@ 2018-05-22 10:45           ` Kevin Traynor
  0 siblings, 0 replies; 19+ messages in thread
From: Kevin Traynor @ 2018-05-22 10:45 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: Aaron Conole, stable, Alejandro Lucero, Ferruh Yigit,
	Eelco Chaudron, Pablo Cascon, Adrien Mazarguil, Thomas Monjalon

On 05/20/2018 08:19 AM, Yuanhan Liu wrote:
> On Mon, May 14, 2018 at 03:23:41PM +0100, Kevin Traynor wrote:
>> Ping Yuanhan - can you clarify question below? Also, can you let us know
> 
> Hi Kevin,
> 
> Sorry for being late: I just have very little time at weekends. I planned
> to handle it last week, but I had to do something else.
> 
> And to your question: yes, they are not pushed yet: I should have done that
> weeks ago though.
> 
> I will push them today.
> 
Hi Yuanhan, thanks - I see them now.

>> when you are planning further backports to 17.11 stable branch?
> 
> I just want to know do you expect something else to be released in 17.11.3?
> If so, could you do a list, just for making sure I don't miss it.
> 

I ran git-log-fixes and it gave a list of 253 post v18.02 fixes in head
of master (not including typo/base drv fixes) that look relevant for
17.11, with another 20 unclear as they are missing 'Fixes:' tag. On the
17.11 branch there's 99 fixes from v17.11.1..HEAD.

There might a few discrepancies in accounting due to different
18.02.0/17.11.1 release dates etc, but at that number it seems like
there are fixes missing - unless they are not suitable for backport for
some other criteria?

I made a list of all fixes that look relevant on master and 17.11
branches, it's quite long, I will send separately.

One unusual case that is needed is a fix for a fix that is not on master
but is on stable branches. The fix is here:
http://dpdk.org/browse/dpdk-stable/commit/?h=18.02&id=e89e323e4ce58f2cbbc7afcc2d5dc7ce66075e81

thanks,
Kevin.

> 	--yliu
>>
>> thanks,
>> Kevin.
>>
>> On 05/08/2018 10:23 AM, Kevin Traynor wrote:
>>> On 05/06/2018 07:34 AM, Yuanhan Liu wrote:
>>>> On Mon, Apr 30, 2018 at 07:02:29PM +0100, Kevin Traynor wrote:
>>>>> On 04/30/2018 06:20 PM, Aaron Conole wrote:
>>>>>> Even when trying to use the Netronome cards via VFIO, there is a
>>>>>> lock file being requested, which requires root access.  This series
>>>>>> fixes an issue with the nfp driver lock release always releasing
>>>>>> 'nfp0' (which isn't a bug in practice for other reasons) as well
>>>>>> as allowing the lock file location to be configured by the application
>>>>>> or user via the HOME environment variable (similar to other resources
>>>>>> in the DPDK framework).
>>>>>>
>>>>>> This series is only applicable to the stable tree because the nfp
>>>>>> driver is completely rewritten for later versions of DPDK.
>>>>>>
>>>>>
>>>>> Hi Yuanhan, I think we would like to get this into the next 17.11 stable
>>>>> release if possible. Not sure when you are merging patches into the tree
>>>>> for it?
>>>>
>>>> Hi,
>>>>
>>>> I have just applied it to dpdk-stable/17.11. It will be part of 17.11.3
>>>> LTS release.
>>>>
>>>
>>> Hi,
>>>
>>> They don't appear to be on the branch? Maybe you have a private 'next'
>>> branch or something in your workflow, but reporting in case something
>>> still needs to be pushed.
>>>
>>> thanks,
>>> Kevin.
>>>
>>>> Thanks.
>>>>
>>>> 	--yliu
>>>>>
>>>>> Kevin.
>>>>>
>>>>>> Aaron Conole (2):
>>>>>>   nfp: unlink the appropriate lock file
>>>>>>   nfp: allow for non-root user
>>>>>>
>>>>>>  drivers/net/nfp/nfp_nfpu.c | 25 +++++++++++++++++++++----
>>>>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>>>>
>>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-05-22 10:45 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-30 17:20 [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Aaron Conole
2018-04-30 17:20 ` [dpdk-stable] [PATCH 1/2] nfp: unlink the appropriate lock file Aaron Conole
2018-04-30 17:20 ` [dpdk-stable] [PATCH 2/2] nfp: allow for non-root user Aaron Conole
2018-05-08 13:09   ` Eelco Chaudron
2018-05-08 13:20     ` Aaron Conole
2018-05-09 17:05   ` Alejandro Lucero
2018-05-09 17:53     ` Aaron Conole
2018-05-09 19:44       ` Alejandro Lucero
2018-05-10 12:00         ` Alejandro Lucero
2018-04-30 18:02 ` [dpdk-stable] [PATCH 0/2] nfp: support non-root user for the Netronome Kevin Traynor
2018-05-06  6:34   ` Yuanhan Liu
2018-05-08  9:23     ` Kevin Traynor
2018-05-14 14:23       ` Kevin Traynor
2018-05-20  7:19         ` Yuanhan Liu
2018-05-22 10:45           ` Kevin Traynor
2018-05-01 10:36 ` Luca Boccassi
2018-05-03 10:30   ` Luca Boccassi
2018-05-03 12:25     ` Alejandro Lucero
2018-05-03 12:57       ` Luca Boccassi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).