diff mbox series

[iproute2-next] rdma: Relax requirement to have PID for HW objects

Message ID 20191002134934.19226-1-leon@kernel.org
State Accepted
Delegated to: David Ahern
Headers show
Series [iproute2-next] rdma: Relax requirement to have PID for HW objects | expand

Commit Message

Leon Romanovsky Oct. 2, 2019, 1:49 p.m. UTC 
From: Leon Romanovsky <leonro@mellanox.com>

RDMA has weak connection between PIDs and HW objects, because
the latter tied to file descriptors for their lifetime management.

The outcome of such connection is that for the following scenario,
the returned PID will be 0 (not-valid):
 1. Create FD and context
 2. Share it with ephemeral child
 3. Create any object and exit that child

This flow was revealed in testing environment and of course real users
are not running such scenario, because it makes no sense at all in RDMA
world.

Let's do two changes in the code to support such workflow anyway:
 1. Remove need to provide PID/kernel name. Code already supports it,
    just need to remove extra validation.
 2. Ball-out in case PID is 0.

Link: https://lore.kernel.org/linux-rdma/20191002123245.18153-2-leon@kernel.org
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 rdma/res-cmid.c | 5 +----
 rdma/res-cq.c   | 5 +----
 rdma/res-mr.c   | 5 +----
 rdma/res-pd.c   | 5 +----
 rdma/res-qp.c   | 5 +----
 rdma/res.c      | 3 +++
 6 files changed, 8 insertions(+), 20 deletions(-)

Comments

David Ahern Oct. 7, 2019, 9:55 p.m. UTC | #1 
On 10/2/19 7:49 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> RDMA has weak connection between PIDs and HW objects, because
> the latter tied to file descriptors for their lifetime management.
> 
> The outcome of such connection is that for the following scenario,
> the returned PID will be 0 (not-valid):
>  1. Create FD and context
>  2. Share it with ephemeral child
>  3. Create any object and exit that child
> 
> This flow was revealed in testing environment and of course real users
> are not running such scenario, because it makes no sense at all in RDMA
> world.
> 
> Let's do two changes in the code to support such workflow anyway:
>  1. Remove need to provide PID/kernel name. Code already supports it,
>     just need to remove extra validation.
>  2. Ball-out in case PID is 0.
> 
> Link: https://lore.kernel.org/linux-rdma/20191002123245.18153-2-leon@kernel.org
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  rdma/res-cmid.c | 5 +----
>  rdma/res-cq.c   | 5 +----
>  rdma/res-mr.c   | 5 +----
>  rdma/res-pd.c   | 5 +----
>  rdma/res-qp.c   | 5 +----
>  rdma/res.c      | 3 +++
>  6 files changed, 8 insertions(+), 20 deletions(-)
> 

applied to iproute2-next
diff mbox series

Patch

diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c
index 0b830088..0ee9c3d4 100644
--- a/rdma/res-cmid.c
+++ b/rdma/res-cmid.c
@@ -120,11 +120,8 @@  static int res_cm_id_line(struct rd *rd, const char *name, int idx,
 	char *comm = NULL;
 
 	if (!nla_line[RDMA_NLDEV_ATTR_RES_STATE] ||
-	    !nla_line[RDMA_NLDEV_ATTR_RES_PS] ||
-	    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
-	     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+	    !nla_line[RDMA_NLDEV_ATTR_RES_PS])
 		return MNL_CB_ERROR;
-	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
 		port = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_PORT_INDEX]);
diff --git a/rdma/res-cq.c b/rdma/res-cq.c
index d2591fbe..6855e798 100644
--- a/rdma/res-cq.c
+++ b/rdma/res-cq.c
@@ -56,11 +56,8 @@  static int res_cq_line(struct rd *rd, const char *name, int idx,
 	uint32_t cqe;
 
 	if (!nla_line[RDMA_NLDEV_ATTR_RES_CQE] ||
-	    !nla_line[RDMA_NLDEV_ATTR_RES_USECNT] ||
-	    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
-	     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+	    !nla_line[RDMA_NLDEV_ATTR_RES_USECNT])
 		return MNL_CB_ERROR;
-	}
 
 	cqe = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_CQE]);
 
diff --git a/rdma/res-mr.c b/rdma/res-mr.c
index f4a24dc1..c1b8069a 100644
--- a/rdma/res-mr.c
+++ b/rdma/res-mr.c
@@ -17,11 +17,8 @@  static int res_mr_line(struct rd *rd, const char *name, int idx,
 	uint32_t mrn = 0;
 	uint32_t pid = 0;
 
-	if (!nla_line[RDMA_NLDEV_ATTR_RES_MRLEN] ||
-	    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
-	     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+	if (!nla_line[RDMA_NLDEV_ATTR_RES_MRLEN])
 		return MNL_CB_ERROR;
-	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_RES_RKEY])
 		rkey = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_RKEY]);
diff --git a/rdma/res-pd.c b/rdma/res-pd.c
index 07c836e8..6e5e4e6b 100644
--- a/rdma/res-pd.c
+++ b/rdma/res-pd.c
@@ -17,11 +17,8 @@  static int res_pd_line(struct rd *rd, const char *name, int idx,
 	uint32_t pdn = 0;
 	uint64_t users;
 
-	if (!nla_line[RDMA_NLDEV_ATTR_RES_USECNT] ||
-	    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
-	     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+	if (!nla_line[RDMA_NLDEV_ATTR_RES_USECNT])
 		return MNL_CB_ERROR;
-	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY])
 		local_dma_lkey = mnl_attr_get_u32(
diff --git a/rdma/res-qp.c b/rdma/res-qp.c
index 954e465d..e30d68ed 100644
--- a/rdma/res-qp.c
+++ b/rdma/res-qp.c
@@ -90,11 +90,8 @@  static int res_qp_line(struct rd *rd, const char *name, int idx,
 	if (!nla_line[RDMA_NLDEV_ATTR_RES_LQPN] ||
 	    !nla_line[RDMA_NLDEV_ATTR_RES_SQ_PSN] ||
 	    !nla_line[RDMA_NLDEV_ATTR_RES_TYPE] ||
-	    !nla_line[RDMA_NLDEV_ATTR_RES_STATE] ||
-	    (!nla_line[RDMA_NLDEV_ATTR_RES_PID] &&
-	     !nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])) {
+	    !nla_line[RDMA_NLDEV_ATTR_RES_STATE])
 		return MNL_CB_ERROR;
-	}
 
 	if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
 		port = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_PORT_INDEX]);
diff --git a/rdma/res.c b/rdma/res.c
index 6003006e..e8607808 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -211,6 +211,9 @@  char *get_task_name(uint32_t pid)
 	char *comm;
 	FILE *f;
 
+	if (!pid)
+		return NULL;
+
 	if (asprintf(&comm, "/proc/%d/comm", pid) < 0)
 		return NULL;