[dpdk-dev,RFC,1/2] nfp: unlink the appropriate lock file

Message ID 20180412222208.11770-2-aconole@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Aaron Conole April 12, 2018, 10:22 p.m. UTC
  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(-)
  

Comments

Alejandro Lucero April 13, 2018, 7:31 a.m. UTC | #1
Although the patch is correct, the truth is there was not option for locks
but "nfp0", because the PMD did not support more than one NFP card.

After commit with CPP support, the PMD can support more than one NFP card,
but this interface is not supported anymore and this file was removed.

is this patch coming from some code revision or from a problem you found?

On Fri, Apr 13, 2018 at 12:22 AM, Aaron Conole <aconole@redhat.com> wrote:

> 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
>
>
  
Aaron Conole April 13, 2018, 1:24 p.m. UTC | #2
Alejandro Lucero <alejandro.lucero@netronome.com> writes:

> Although the patch is correct, the truth is there was not option for locks but "nfp0", because
> the PMD did not support more than one NFP card. 
>
> After commit with CPP support, the PMD can support more than one NFP card, but this
> interface is not supported anymore and this file was removed.
>
> is this patch coming from some code revision or from a problem you found?

Just a code review - we were reported a problem initializing the driver,
and in looking at the lock/unlock code, it seems that it was always
releasing the nfp0 lock.  Good to know that in practice it likely
wouldn't cause real harm.  I intend to resubmit this as is.

Thanks, Alejandro!

> On Fri, Apr 13, 2018 at 12:22 AM, Aaron Conole <aconole@redhat.com> wrote:
>
>  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
  

Patch

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;
 }