[dpdk-dev] [PATCH] net/nfp: fix lock file usage

[Alejandro Lucero]

On Thu, May 24, 2018 at 3:15 PM, Ferruh Yigit <ferruh.yigit at intel.com>
wrote:

> On 5/24/2018 3:13 PM, Ferruh Yigit wrote:
> > On 5/24/2018 3:02 PM, Alejandro Lucero wrote:
> >>
> >>
> >> On Thu, May 24, 2018 at 11:18 AM, Ferruh Yigit <ferruh.yigit at intel.com
> >> <mailto:ferruh.yigit at intel.com>> wrote:
> >>
> >>     On 5/23/2018 5:50 PM, Alejandro Lucero wrote:
> >>     >
> >>     >
> >>     > On Wed, May 23, 2018 at 4:57 PM, Ferruh Yigit <
> ferruh.yigit at intel.com <mailto:ferruh.yigit at intel.com>
> >>     > <mailto:ferruh.yigit at intel.com <mailto:ferruh.yigit at intel.com>>>
> wrote:
> >>     >
> >>     >     On 5/23/2018 1:28 PM, Alejandro Lucero wrote:
> >>     >     > DPDK apps can be executed as non-root users but current NFP
> lock
> >>     >     > file for avoiding concurrent accesses to CPP interface is
> precluding
> >>     >     > this option or requires to modify system file permissions.
> >>     >     >
> >>     >     > When the NFP device is bound to VFIO, this driver does not
> allow this
> >>     >     > concurrent access, so the lock file is not required at all.
> >>     >     >
> >>     >     > OVS-DPDK as executed in RedHat distributions is the main
> NFP user
> >>     >     > needing this fix.
> >>     >     >
> >>     >     > Fixes: c7e9729da6b5 ("net/nfp: support CPP")
> >>     >     >
> >>     >     > Signed-off-by: Alejandro Lucero <
> alejandro.lucero at netronome.com
> >>     <mailto:alejandro.lucero at netronome.com>
> >>     <mailto:alejandro.lucero at netronome.com <mailto:alejandro.lucero@
> netronome.com>>>
> >>     >
> >>     >     Hi Alejandro,
> >>     >
> >>     >     As far as I understand this is to fix a common use case for
> nfp, but it looks
> >>     >     like there is already a workaround and only for non-root
> users.
> >>     >
> >>     >
> >>     > There is a patch submitted to stable versions because this lock
> was also with
> >>     > the old NSPU interface, but as far as I know, there is no patch
> yet for the
> >>     > current upstream tip.
> >>     >
> >>     >
> >>     >
> >>     >     What is the priority of the patch, only critical but fixes
> allowed at this
> >>     >     point, can we push this one to next release?
> >>     >
> >>     >
> >>     > This is critical for us because RedHat wants to support OVS with
> our card, and
> >>     > when OVS-DPDK is used, this problem is precluding non-root users
> to execute
> >>     > OVS-DPDK.
> >>
> >>     What exactly this lock for? Does it to prevent multiple primary
> process to
> >>     access CPP interface?
> >>
> >>     If so this is the know limitation in DPDK, not two separate process
> can driver
> >>     same hardware, this is valid for all devices, why adding a lock
> unique to nfp?
> >>
> >>
> >> Time ago I had, by mistake, two different DPDK processes using same
> device, and
> >> with UIO, there is no one avoiding this.
> >>
> >> You can bound a device to UIO, igb_uio, and then use two different
> processes
> >> opening the /dev/uiox file, and it works.
> >
> > But this is not anything specific to nfp, isn't it?
>
> Or let me ask something else, is this a fix for ovs-dpdk regular use-case
> with
> nfp? Or this is just an extra protection in case multiple process may try
> to use
> the NIC. If second, why it is critical?
>
>
I think any device bound to UIO could end up being used by two different
DPDK processes. So this is a protection against that possibility, because
accessing the NFP through the new CPP interface could make the NFP a brick
even it could crash the system.

RH is configuring OVS-DPDK for running as non-root, and the lock was
precluding this because it is set at /var/lock which a non-root user has
not access by default. This patch solves the problem, because when using
the device with VFIO, that lock is not necessary.  And with UIO, the lock
is needed and because it is not possible to run DPDK apps with UIO as
non-root, the lock path is fine.




> >
> >>
> >> The VFIO driver does avoid this situation, but this lock is required
> for UIO.
> >>
> >
>
>