[dpdk-dev,RFC,2/2] nfp: allow for non-root user
Checks
Commit Message
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.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
drivers/net/nfp/nfp_nfpu.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
Comments
Again, this patch is correct, but because NFP PMD needs to access
/sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these
files have just read/write accesses for root, I do not know if this is
really necessary.
Being honest, I have not used a DPDK app with NFP PMD and not being root.
Does it work with non-root users and other PMDs with same requirements
regarding sysfs resource files?
On Fri, Apr 13, 2018 at 12:22 AM, 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.
>
> 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
>
>
Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> Again, this patch is correct, but because NFP PMD needs to access
> /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these files have just
> read/write accesses for root, I do not know if this is really necessary.
>
> Being honest, I have not used a DPDK app with NFP PMD and not being root. Does it work
> with non-root users and other PMDs with same requirements regarding sysfs resource files?
We do run as non-root user definitely with Intel PMDs.
I'm not very sure about other vendors, but I think mlx pmd runs as
non-root user (and it was modified to move off of sysfs for that
reason[1]).
I'll continue to push for more information from the testing side to find
out though.
[1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> On Fri, Apr 13, 2018 at 12:22 AM, 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.
>
> 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
On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com> wrote:
> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>
> > Again, this patch is correct, but because NFP PMD needs to access
> > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these
> files have just
> > read/write accesses for root, I do not know if this is really necessary.
> >
> > Being honest, I have not used a DPDK app with NFP PMD and not being
> root. Does it work
> > with non-root users and other PMDs with same requirements regarding
> sysfs resource files?
>
> We do run as non-root user definitely with Intel PMDs.
>
> I'm not very sure about other vendors, but I think mlx pmd runs as
> non-root user (and it was modified to move off of sysfs for that
> reason[1]).
>
>
It is possible to not rely on sysfs resource files if device is attached to
VFIO, but I think that is a must with UIO.
> I'll continue to push for more information from the testing side to find
> out though.
>
> [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
>
> > On Fri, Apr 13, 2018 at 12:22 AM, 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.
> >
> > 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
>
I have seen that VFIO also requires explicitly to set the right permissions
for non-root users to VFIO groups under /dev/vfio.
I assume then that running OVS or other DPDK apps as non-root is possible,
although requiring those explicit permissions changes, and therefore this
patch is necessary.
Adding stable@ and Thomas for discussing how can this be added to stable
DPDK versions even if this is not going to be a patch for current DPDK
version.
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero <
alejandro.lucero@netronome.com> wrote:
>
>
> On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com> wrote:
>
>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>
>> > Again, this patch is correct, but because NFP PMD needs to access
>> > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and
>> these files have just
>> > read/write accesses for root, I do not know if this is really necessary.
>> >
>> > Being honest, I have not used a DPDK app with NFP PMD and not being
>> root. Does it work
>> > with non-root users and other PMDs with same requirements regarding
>> sysfs resource files?
>>
>> We do run as non-root user definitely with Intel PMDs.
>>
>> I'm not very sure about other vendors, but I think mlx pmd runs as
>> non-root user (and it was modified to move off of sysfs for that
>> reason[1]).
>>
>>
> It is possible to not rely on sysfs resource files if device is attached
> to VFIO, but I think that is a must with UIO.
>
>
>
>> I'll continue to push for more information from the testing side to find
>> out though.
>>
>> [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
>>
>> > On Fri, Apr 13, 2018 at 12:22 AM, 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.
>> >
>> > 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
>>
>
>
I was just wondering, if device device PCI sysfs resource files or VFIO
group /dev files require to change permissions for non-root users, does it
not make sense to adjust also /var/lock in the system?
On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero <
alejandro.lucero@netronome.com> wrote:
> I have seen that VFIO also requires explicitly to set the right
> permissions for non-root users to VFIO groups under /dev/vfio.
>
> I assume then that running OVS or other DPDK apps as non-root is possible,
> although requiring those explicit permissions changes, and therefore this
> patch is necessary.
>
> Adding stable@ and Thomas for discussing how can this be added to stable
> DPDK versions even if this is not going to be a patch for current DPDK
> version.
>
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>
>
> On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero <
> alejandro.lucero@netronome.com> wrote:
>
>>
>>
>> On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com> wrote:
>>
>>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>>
>>> > Again, this patch is correct, but because NFP PMD needs to access
>>> > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and
>>> these files have just
>>> > read/write accesses for root, I do not know if this is really
>>> necessary.
>>> >
>>> > Being honest, I have not used a DPDK app with NFP PMD and not being
>>> root. Does it work
>>> > with non-root users and other PMDs with same requirements regarding
>>> sysfs resource files?
>>>
>>> We do run as non-root user definitely with Intel PMDs.
>>>
>>> I'm not very sure about other vendors, but I think mlx pmd runs as
>>> non-root user (and it was modified to move off of sysfs for that
>>> reason[1]).
>>>
>>>
>> It is possible to not rely on sysfs resource files if device is attached
>> to VFIO, but I think that is a must with UIO.
>>
>>
>>
>>> I'll continue to push for more information from the testing side to find
>>> out though.
>>>
>>> [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
>>>
>>> > On Fri, Apr 13, 2018 at 12:22 AM, 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.
>>> >
>>> > 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
>>>
>>
>>
>
17/04/2018 17:44, Alejandro Lucero:
> Adding stable@ and Thomas for discussing how can this be added to stable
> DPDK versions even if this is not going to be a patch for current DPDK
> version.
I don't understand.
This patch won't enter in 18.05?
Why do you think this patch is candidate for stable but not master?
On Tue, Apr 17, 2018 at 4:54 PM, Thomas Monjalon <thomas@monjalon.net>
wrote:
> 17/04/2018 17:44, Alejandro Lucero:
> > Adding stable@ and Thomas for discussing how can this be added to stable
> > DPDK versions even if this is not going to be a patch for current DPDK
> > version.
>
> I don't understand.
> This patch won't enter in 18.05?
> Why do you think this patch is candidate for stable but not master?
>
>
>
Because all that code has been removed between 18.02 and 18.05:
http://dpdk.org/ml/archives/dev/2018-March/093655.html
I guess this had not happen yet, and stable versions only pull patches from
vanilla. Am I right?
17/04/2018 18:24, Alejandro Lucero:
> On Tue, Apr 17, 2018 at 4:54 PM, Thomas Monjalon <thomas@monjalon.net>
> wrote:
>
> > 17/04/2018 17:44, Alejandro Lucero:
> > > Adding stable@ and Thomas for discussing how can this be added to stable
> > > DPDK versions even if this is not going to be a patch for current DPDK
> > > version.
> >
> > I don't understand.
> > This patch won't enter in 18.05?
> > Why do you think this patch is candidate for stable but not master?
> >
> Because all that code has been removed between 18.02 and 18.05:
>
> http://dpdk.org/ml/archives/dev/2018-March/093655.html
>
> I guess this had not happen yet, and stable versions only pull patches from
> vanilla. Am I right?
Yes patches are cherry-picked from master.
But when the backport is too much difficult, a patch can be sent directly
to stable@dpdk.org.
I don't know whether it already happened to fix a removed code.
You need to make sure that the maintainers of stable branches will pick it.
There is no special process, it's all a matter of communication :)
Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> I was just wondering, if device device PCI sysfs resource files or VFIO group /dev files require to change
> permissions for non-root users, does it not make sense to adjust also /var/lock in the system?
For the /dev, we use udev rules - so the correct individual vfio device
files get assigned the correct permissions. No such mechanism exists
for /var/lock as far as I can tell.
Ex. see:
https://github.com/openvswitch/ovs/blob/master/rhel/usr_lib_udev_rules.d_91-vfio.rules
Maybe something similar exists that we could use to generate the lock
file automatically?
> On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero <alejandro.lucero@netronome.com> wrote:
>
> I have seen that VFIO also requires explicitly to set the right permissions for non-root users to VFIO
> groups under /dev/vfio.
>
> I assume then that running OVS or other DPDK apps as non-root is possible, although requiring
> those explicit permissions changes, and therefore this patch is necessary.
>
> Adding stable@ and Thomas for discussing how can this be added to stable DPDK versions even if
> this is not going to be a patch for current DPDK version.
>
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>
> On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero <alejandro.lucero@netronome.com> wrote:
>
> On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com> wrote:
>
> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>
> > Again, this patch is correct, but because NFP PMD needs to access
> > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these files have
> just
> > read/write accesses for root, I do not know if this is really necessary.
> >
> > Being honest, I have not used a DPDK app with NFP PMD and not being root. Does it
> work
> > with non-root users and other PMDs with same requirements regarding sysfs resource
> files?
>
> We do run as non-root user definitely with Intel PMDs.
>
> I'm not very sure about other vendors, but I think mlx pmd runs as
> non-root user (and it was modified to move off of sysfs for that
> reason[1]).
>
> It is possible to not rely on sysfs resource files if device is attached to VFIO, but I think that is a
> must with UIO.
>
>
> I'll continue to push for more information from the testing side to find
> out though.
>
> [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
>
> > On Fri, Apr 13, 2018 at 12:22 AM, 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.
> >
> > 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
On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole <aconole@redhat.com> wrote:
> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>
> > I was just wondering, if device device PCI sysfs resource files or VFIO
> group /dev files require to change
> > permissions for non-root users, does it not make sense to adjust also
> /var/lock in the system?
>
> For the /dev, we use udev rules - so the correct individual vfio device
> files get assigned the correct permissions. No such mechanism exists
> for /var/lock as far as I can tell.
>
> Ex. see:
>
> https://github.com/openvswitch/ovs/blob/master/
> rhel/usr_lib_udev_rules.d_91-vfio.rules
>
> Maybe something similar exists that we could use to generate the lock
> file automatically?
>
What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
Is RH forcing OVS DPDK to only work if the host has IOMMU support?
>
> > On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero <
> alejandro.lucero@netronome.com> wrote:
> >
> > I have seen that VFIO also requires explicitly to set the right
> permissions for non-root users to VFIO
> > groups under /dev/vfio.
> >
> > I assume then that running OVS or other DPDK apps as non-root is
> possible, although requiring
> > those explicit permissions changes, and therefore this patch is
> necessary.
> >
> > Adding stable@ and Thomas for discussing how can this be added to
> stable DPDK versions even if
> > this is not going to be a patch for current DPDK version.
> >
> > Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> >
> > On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero <
> alejandro.lucero@netronome.com> wrote:
> >
> > On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com>
> wrote:
> >
> > Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> >
> > > Again, this patch is correct, but because NFP PMD needs to access
> > > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and
> these files have
> > just
> > > read/write accesses for root, I do not know if this is really
> necessary.
> > >
> > > Being honest, I have not used a DPDK app with NFP PMD and not being
> root. Does it
> > work
> > > with non-root users and other PMDs with same requirements regarding
> sysfs resource
> > files?
> >
> > We do run as non-root user definitely with Intel PMDs.
> >
> > I'm not very sure about other vendors, but I think mlx pmd runs as
> > non-root user (and it was modified to move off of sysfs for that
> > reason[1]).
> >
> > It is possible to not rely on sysfs resource files if device is
> attached to VFIO, but I think that is a
> > must with UIO.
> >
> >
> > I'll continue to push for more information from the testing side to find
> > out though.
> >
> > [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> >
> > > On Fri, Apr 13, 2018 at 12:22 AM, 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.
> > >
> > > 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
>
Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole <aconole@redhat.com> wrote:
>
> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>
> > I was just wondering, if device device PCI sysfs resource files or VFIO group /dev files
> require to change
> > permissions for non-root users, does it not make sense to adjust also /var/lock in the
> system?
>
> For the /dev, we use udev rules - so the correct individual vfio device
> files get assigned the correct permissions. No such mechanism exists
> for /var/lock as far as I can tell.
>
> Ex. see:
>
> https://github.com/openvswitch/ovs/blob/master/rhel/usr_lib_udev_rules.d_91-vfio.rules
>
>
> Maybe something similar exists that we could use to generate the lock
> file automatically?
>
> What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
>
> Is RH forcing OVS DPDK to only work if the host has IOMMU support?
Yes.
> > On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero
> <alejandro.lucero@netronome.com> wrote:
> >
> > I have seen that VFIO also requires explicitly to set the right permissions for non-root
> users to VFIO
> > groups under /dev/vfio.
> >
> > I assume then that running OVS or other DPDK apps as non-root is possible,
> although requiring
> > those explicit permissions changes, and therefore this patch is necessary.
> >
> > Adding stable@ and Thomas for discussing how can this be added to stable DPDK
> versions even if
> > this is not going to be a patch for current DPDK version.
> >
> > Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> >
> > On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero
> <alejandro.lucero@netronome.com> wrote:
> >
> > On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com> wrote:
> >
> > Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> >
> > > Again, this patch is correct, but because NFP PMD needs to access
> > > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and these files
> have
> > just
> > > read/write accesses for root, I do not know if this is really necessary.
> > >
> > > Being honest, I have not used a DPDK app with NFP PMD and not being root. Does
> it
> > work
> > > with non-root users and other PMDs with same requirements regarding sysfs
> resource
> > files?
> >
> > We do run as non-root user definitely with Intel PMDs.
> >
> > I'm not very sure about other vendors, but I think mlx pmd runs as
> > non-root user (and it was modified to move off of sysfs for that
> > reason[1]).
> >
> > It is possible to not rely on sysfs resource files if device is attached to VFIO, but I
> think that is a
> > must with UIO.
> >
> >
> > I'll continue to push for more information from the testing side to find
> > out though.
> >
> > [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> >
> > > On Fri, Apr 13, 2018 at 12:22 AM, 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.
> > >
> > > 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
On Wed, Apr 18, 2018 at 1:32 PM, Aaron Conole <aconole@redhat.com> wrote:
> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>
> > On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole <aconole@redhat.com>
> wrote:
> >
> > Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> >
> > > I was just wondering, if device device PCI sysfs resource files or
> VFIO group /dev files
> > require to change
> > > permissions for non-root users, does it not make sense to adjust also
> /var/lock in the
> > system?
> >
> > For the /dev, we use udev rules - so the correct individual vfio device
> > files get assigned the correct permissions. No such mechanism exists
> > for /var/lock as far as I can tell.
> >
> > Ex. see:
> >
> > https://github.com/openvswitch/ovs/blob/master/
> rhel/usr_lib_udev_rules.d_91-vfio.rules
> >
> >
> > Maybe something similar exists that we could use to generate the lock
> > file automatically?
> >
> > What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
> >
> > Is RH forcing OVS DPDK to only work if the host has IOMMU support?
>
> Yes.
>
Ok then. It makes sense now to apply this patch to stable versions.
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>
> > > On Tue, Apr 17, 2018 at 4:44 PM, Alejandro Lucero
> > <alejandro.lucero@netronome.com> wrote:
> > >
> > > I have seen that VFIO also requires explicitly to set the right
> permissions for non-root
> > users to VFIO
> > > groups under /dev/vfio.
> > >
> > > I assume then that running OVS or other DPDK apps as non-root is
> possible,
> > although requiring
> > > those explicit permissions changes, and therefore this patch is
> necessary.
> > >
> > > Adding stable@ and Thomas for discussing how can this be added to
> stable DPDK
> > versions even if
> > > this is not going to be a patch for current DPDK version.
> > >
> > > Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > >
> > > On Fri, Apr 13, 2018 at 4:31 PM, Alejandro Lucero
> > <alejandro.lucero@netronome.com> wrote:
> > >
> > > On Fri, Apr 13, 2018 at 2:31 PM, Aaron Conole <aconole@redhat.com>
> wrote:
> > >
> > > Alejandro Lucero <alejandro.lucero@netronome.com> writes:
> > >
> > > > Again, this patch is correct, but because NFP PMD needs to access
> > > > /sys/bus/pci/devices/$DEVICE_PCI_STRING/resource$RESOURCE_ID, and
> these files
> > have
> > > just
> > > > read/write accesses for root, I do not know if this is really
> necessary.
> > > >
> > > > Being honest, I have not used a DPDK app with NFP PMD and not
> being root. Does
> > it
> > > work
> > > > with non-root users and other PMDs with same requirements
> regarding sysfs
> > resource
> > > files?
> > >
> > > We do run as non-root user definitely with Intel PMDs.
> > >
> > > I'm not very sure about other vendors, but I think mlx pmd runs as
> > > non-root user (and it was modified to move off of sysfs for that
> > > reason[1]).
> > >
> > > It is possible to not rely on sysfs resource files if device is
> attached to VFIO, but I
> > think that is a
> > > must with UIO.
> > >
> > >
> > > I'll continue to push for more information from the testing side to
> find
> > > out though.
> > >
> > > [1]: http://dpdk.org/ml/archives/dev/2018-February/090586.html
> > >
> > > > On Fri, Apr 13, 2018 at 12:22 AM, 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.
> > > >
> > > > 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
>
On 4/19/2018 7:05 AM, Alejandro Lucero wrote:
> On Wed, Apr 18, 2018 at 1:32 PM, Aaron Conole <aconole@redhat.com> wrote:
>
>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>
>>> On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole <aconole@redhat.com>
>> wrote:
>>>
>>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>>
>>> > I was just wondering, if device device PCI sysfs resource files or
>> VFIO group /dev files
>>> require to change
>>> > permissions for non-root users, does it not make sense to adjust also
>> /var/lock in the
>>> system?
>>>
>>> For the /dev, we use udev rules - so the correct individual vfio device
>>> files get assigned the correct permissions. No such mechanism exists
>>> for /var/lock as far as I can tell.
>>>
>>> Ex. see:
>>>
>>> https://github.com/openvswitch/ovs/blob/master/
>> rhel/usr_lib_udev_rules.d_91-vfio.rules
>>>
>>>
>>> Maybe something similar exists that we could use to generate the lock
>>> file automatically?
>>>
>>> What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
>>>
>>> Is RH forcing OVS DPDK to only work if the host has IOMMU support?
>>
>> Yes.
>>
>
> Ok then. It makes sense now to apply this patch to stable versions.
>
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Since the target is the stable tree, I will drop them from patchwork as not
applicable.
Can you please send v1 of the patch to the stable mail list, it can be good idea
to cc stable maintainers as well.
Ferruh Yigit <ferruh.yigit@intel.com> writes:
> On 4/19/2018 7:05 AM, Alejandro Lucero wrote:
>> On Wed, Apr 18, 2018 at 1:32 PM, Aaron Conole <aconole@redhat.com> wrote:
>>
>>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>>
>>>> On Tue, Apr 17, 2018 at 8:19 PM, Aaron Conole <aconole@redhat.com>
>>> wrote:
>>>>
>>>> Alejandro Lucero <alejandro.lucero@netronome.com> writes:
>>>>
>>>> > I was just wondering, if device device PCI sysfs resource files or
>>> VFIO group /dev files
>>>> require to change
>>>> > permissions for non-root users, does it not make sense to adjust also
>>> /var/lock in the
>>>> system?
>>>>
>>>> For the /dev, we use udev rules - so the correct individual vfio device
>>>> files get assigned the correct permissions. No such mechanism exists
>>>> for /var/lock as far as I can tell.
>>>>
>>>> Ex. see:
>>>>
>>>> https://github.com/openvswitch/ovs/blob/master/
>>> rhel/usr_lib_udev_rules.d_91-vfio.rules
>>>>
>>>>
>>>> Maybe something similar exists that we could use to generate the lock
>>>> file automatically?
>>>>
>>>> What about /sysfs/bus/pci/device/$PCI_DEV/resource file?
>>>>
>>>> Is RH forcing OVS DPDK to only work if the host has IOMMU support?
>>>
>>> Yes.
>>>
>>
>> Ok then. It makes sense now to apply this patch to stable versions.
>>
>> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>
> Since the target is the stable tree, I will drop them from patchwork as not
> applicable.
>
> Can you please send v1 of the patch to the stable mail list, it can be good idea
> to cc stable maintainers as well.
Will do. I'll spin into a proper series and submit next week.
Sorry for the extra noise, Ferruh.
@@ -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;
}