[PATCH for-next 00/20] Misc update for rtrs

[PATCH for-next 00/20] Misc update for rtrs

[Gioh Kim]

Hi Jason, hi Doug,

Please consider to include following changes to the next merge window.
It contains:
- Patch 01 ~ 12: Typical code refactoring patches
- Patch 13: Requested by Jason
https://www.spinics.net/lists/linux-rdma/msg102009.html
- Patch 14 ~ 20: Bug fixes

Dima Stepanov (1):
  RDMA/rtrs: Use strscpy instead of strlcpy

Gioh Kim (8):
  RDMA/rtrs-clt: Remove MAX_SESS_QUEUE_DEPTH from rtrs_send_sess_info
  RDMA/rtrs-clt: No need to check queue_depth when receiving
  RDMA/rtrs: Change MAX_SESS_QUEUE_DEPTH
  RDMA/rtrs: Define MIN_CHUNK_SIZE
  RDMA/rtrs: Do not reset hb_missed_max after re-connection
  RDMA/rtrs-srv: Duplicated session name is not allowed
  RDMA/rtrs-srv: Fix memory leak of unfreed rtrs_srv_stats object
  RDMA/rtrs-clt: Fix memory leak of not-freed sess->stats and
    stats->pcpu_stats

Guoqing Jiang (6):
  RDMA/rtrs-srv: Kill reject_w_econnreset label
  RDMA/rtrs-srv: Clean up the code in __rtrs_srv_change_state
  RDMA/rtrs-clt: Kill rtrs_clt_{start,stop}_hb
  RDMA/rtrs-clt: Kill rtrs_clt_disconnect_from_sysfs
  RDMA/rtrs-srv: Kill __rtrs_srv_change_state
  RDMA/rtrs-clt: Remove redundant 'break'

Jack Wang (1):
  RDMA/rtrs-srv: Fix memory leak when having multiple sessions

Md Haris Iqbal (4):
  RDMA/rtrs-srv: Add error messages for cases when failing RDMA
    connection
  RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its
    stats
  RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight
  RDMA/rtrs-clt: Check if the queue_depth has changed during a
    reconnection

 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c |   5 +-
 drivers/infiniband/ulp/rtrs/rtrs-clt.c       |  59 ++++---
 drivers/infiniband/ulp/rtrs/rtrs-clt.h       |   2 +-
 drivers/infiniband/ulp/rtrs/rtrs-pri.h       |  14 +-
 drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c |  12 +-
 drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c |   1 +
 drivers/infiniband/ulp/rtrs/rtrs-srv.c       | 163 +++++++++++--------
 drivers/infiniband/ulp/rtrs/rtrs-srv.h       |   4 +-
 drivers/infiniband/ulp/rtrs/rtrs.c           |   1 -
 9 files changed, 144 insertions(+), 117 deletions(-)

-- 
2.25.1

[PATCH for-next 01/20] RDMA/rtrs-srv: Kill reject_w_econnreset label

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

We can goto reject_w_err label after initialize err with -ECONNRESET.

Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 740cafffb7ef..3d09d01e34b4 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1780,33 +1780,33 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 
 	u16 version, con_num, cid;
 	u16 recon_cnt;
-	int err;
+	int err = -ECONNRESET;
 
 	if (len < sizeof(*msg)) {
 		pr_err("Invalid RTRS connection request\n");
-		goto reject_w_econnreset;
+		goto reject_w_err;
 	}
 	if (le16_to_cpu(msg->magic) != RTRS_MAGIC) {
 		pr_err("Invalid RTRS magic\n");
-		goto reject_w_econnreset;
+		goto reject_w_err;
 	}
 	version = le16_to_cpu(msg->version);
 	if (version >> 8 != RTRS_PROTO_VER_MAJOR) {
 		pr_err("Unsupported major RTRS version: %d, expected %d\n",
 		       version >> 8, RTRS_PROTO_VER_MAJOR);
-		goto reject_w_econnreset;
+		goto reject_w_err;
 	}
 	con_num = le16_to_cpu(msg->cid_num);
 	if (con_num > 4096) {
 		/* Sanity check */
 		pr_err("Too many connections requested: %d\n", con_num);
-		goto reject_w_econnreset;
+		goto reject_w_err;
 	}
 	cid = le16_to_cpu(msg->cid);
 	if (cid >= con_num) {
 		/* Sanity check */
 		pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
-		goto reject_w_econnreset;
+		goto reject_w_err;
 	}
 	recon_cnt = le16_to_cpu(msg->recon_cnt);
 	srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
@@ -1826,7 +1826,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 			rtrs_err(s, "Session in wrong state: %s\n",
 				  rtrs_srv_state_str(sess->state));
 			mutex_unlock(&srv->paths_mutex);
-			goto reject_w_econnreset;
+			goto reject_w_err;
 		}
 		/*
 		 * Sanity checks
@@ -1835,13 +1835,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 			rtrs_err(s, "Incorrect request: %d, %d\n",
 				  cid, con_num);
 			mutex_unlock(&srv->paths_mutex);
-			goto reject_w_econnreset;
+			goto reject_w_err;
 		}
 		if (s->con[cid]) {
 			rtrs_err(s, "Connection already exists: %d\n",
 				  cid);
 			mutex_unlock(&srv->paths_mutex);
-			goto reject_w_econnreset;
+			goto reject_w_err;
 		}
 	} else {
 		sess = __alloc_sess(srv, cm_id, con_num, recon_cnt,
@@ -1882,9 +1882,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 reject_w_err:
 	return rtrs_rdma_do_reject(cm_id, err);
 
-reject_w_econnreset:
-	return rtrs_rdma_do_reject(cm_id, -ECONNRESET);
-
 close_and_return_err:
 	mutex_unlock(&srv->paths_mutex);
 	close_sess(sess);
-- 
2.25.1

[PATCH for-next 02/20] RDMA/rtrs-clt: Remove MAX_SESS_QUEUE_DEPTH from rtrs_send_sess_info

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

Client receives queue_depth value from server. There is no need
to use MAX_SESS_QUEUE_DEPTH value.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 5e94686bb22d..930a1b496f84 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2455,7 +2455,7 @@ static int rtrs_send_sess_info(struct rtrs_clt_sess *sess)
 	int err;
 
 	rx_sz  = sizeof(struct rtrs_msg_info_rsp);
-	rx_sz += sizeof(u64) * MAX_SESS_QUEUE_DEPTH;
+	rx_sz += sizeof(struct rtrs_sg_desc) * sess->queue_depth;
 
 	tx_iu = rtrs_iu_alloc(1, sizeof(struct rtrs_msg_info_req), GFP_KERNEL,
 			       sess->s.dev->ib_dev, DMA_TO_DEVICE,
-- 
2.25.1

[PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

The queue_depth size is sent from server and
server already checks validity of the value.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 930a1b496f84..0c828ea0f500 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1772,12 +1772,6 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
 	}
 	if (con->c.cid == 0) {
 		queue_depth = le16_to_cpu(msg->queue_depth);
-
-		if (queue_depth > MAX_SESS_QUEUE_DEPTH) {
-			rtrs_err(clt, "Invalid RTRS message: queue=%d\n",
-				  queue_depth);
-			return -ECONNRESET;
-		}
 		if (!sess->rbufs || sess->queue_depth < queue_depth) {
 			kfree(sess->rbufs);
 			sess->rbufs = kcalloc(queue_depth, sizeof(*sess->rbufs),
-- 
2.25.1

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Leon Romanovsky]

On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> 
> The queue_depth size is sent from server and
> server already checks validity of the value.

Do you trust server? What will be if server is not reliable and sends
garbage?

Thanks

> 
> Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 930a1b496f84..0c828ea0f500 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -1772,12 +1772,6 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
>  	}
>  	if (con->c.cid == 0) {
>  		queue_depth = le16_to_cpu(msg->queue_depth);
> -
> -		if (queue_depth > MAX_SESS_QUEUE_DEPTH) {
> -			rtrs_err(clt, "Invalid RTRS message: queue=%d\n",
> -				  queue_depth);
> -			return -ECONNRESET;
> -		}
>  		if (!sess->rbufs || sess->queue_depth < queue_depth) {
>  			kfree(sess->rbufs);
>  			sess->rbufs = kcalloc(queue_depth, sizeof(*sess->rbufs),
> -- 
> 2.25.1
> 

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Haris Iqbal]

On Sun, May 9, 2021 at 1:24 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> > From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> >
> > The queue_depth size is sent from server and
> > server already checks validity of the value.
>
> Do you trust server? What will be if server is not reliable and sends
> garbage?

Hi Leon,

The server code checks for the queue_depth before sending. If the
server is really running malicious code, then the queue_depth is the
last thing that the client needs to worry about.

>
> Thanks
>
> >
> > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > ---
> >  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 ------
> >  1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > index 930a1b496f84..0c828ea0f500 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > @@ -1772,12 +1772,6 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
> >       }
> >       if (con->c.cid == 0) {
> >               queue_depth = le16_to_cpu(msg->queue_depth);
> > -
> > -             if (queue_depth > MAX_SESS_QUEUE_DEPTH) {
> > -                     rtrs_err(clt, "Invalid RTRS message: queue=%d\n",
> > -                               queue_depth);
> > -                     return -ECONNRESET;
> > -             }
> >               if (!sess->rbufs || sess->queue_depth < queue_depth) {
> >                       kfree(sess->rbufs);
> >                       sess->rbufs = kcalloc(queue_depth, sizeof(*sess->rbufs),
> > --
> > 2.25.1
> >

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Leon Romanovsky]

On Mon, May 10, 2021 at 01:00:33PM +0200, Haris Iqbal wrote:
> On Sun, May 9, 2021 at 1:24 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> > > From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> > >
> > > The queue_depth size is sent from server and
> > > server already checks validity of the value.
> >
> > Do you trust server? What will be if server is not reliable and sends
> > garbage?
> 
> Hi Leon,
> 
> The server code checks for the queue_depth before sending. If the
> server is really running malicious code, then the queue_depth is the
> last thing that the client needs to worry about.

Like what? for an example?

Thanks

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Haris Iqbal]

On Mon, May 10, 2021 at 1:53 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 10, 2021 at 01:00:33PM +0200, Haris Iqbal wrote:
> > On Sun, May 9, 2021 at 1:24 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> > > > From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> > > >
> > > > The queue_depth size is sent from server and
> > > > server already checks validity of the value.
> > >
> > > Do you trust server? What will be if server is not reliable and sends
> > > garbage?
> >
> > Hi Leon,
> >
> > The server code checks for the queue_depth before sending. If the
> > server is really running malicious code, then the queue_depth is the
> > last thing that the client needs to worry about.
>
> Like what? for an example?

Like accessing compromised block devices. If the queue_depth is
garbage, the client would fail at allocation with ENOMEM; thats it.

>
> Thanks

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Leon Romanovsky]

On Mon, May 10, 2021 at 02:06:55PM +0200, Haris Iqbal wrote:
> On Mon, May 10, 2021 at 1:53 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Mon, May 10, 2021 at 01:00:33PM +0200, Haris Iqbal wrote:
> > > On Sun, May 9, 2021 at 1:24 PM Leon Romanovsky <leon@kernel.org> wrote:
> > > >
> > > > On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> > > > > From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> > > > >
> > > > > The queue_depth size is sent from server and
> > > > > server already checks validity of the value.
> > > >
> > > > Do you trust server? What will be if server is not reliable and sends
> > > > garbage?
> > >
> > > Hi Leon,
> > >
> > > The server code checks for the queue_depth before sending. If the
> > > server is really running malicious code, then the queue_depth is the
> > > last thing that the client needs to worry about.
> >
> > Like what? for an example?
> 
> Like accessing compromised block devices. If the queue_depth is
> garbage, the client would fail at allocation with ENOMEM; thats it.

The client will get wrong data, check it and discard. The case of ENOMEM triggered
by remote side is different. It can cause to DDOS on the client.

Thanks

> 
> >
> > Thanks

Re: [PATCH for-next 03/20] RDMA/rtrs-clt: No need to check queue_depth when receiving

[Haris Iqbal]

On Mon, May 10, 2021 at 2:18 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 10, 2021 at 02:06:55PM +0200, Haris Iqbal wrote:
> > On Mon, May 10, 2021 at 1:53 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Mon, May 10, 2021 at 01:00:33PM +0200, Haris Iqbal wrote:
> > > > On Sun, May 9, 2021 at 1:24 PM Leon Romanovsky <leon@kernel.org> wrote:
> > > > >
> > > > > On Mon, May 03, 2021 at 01:48:01PM +0200, Gioh Kim wrote:
> > > > > > From: Gioh Kim <gi-oh.kim@cloud.ionos.com>
> > > > > >
> > > > > > The queue_depth size is sent from server and
> > > > > > server already checks validity of the value.
> > > > >
> > > > > Do you trust server? What will be if server is not reliable and sends
> > > > > garbage?
> > > >
> > > > Hi Leon,
> > > >
> > > > The server code checks for the queue_depth before sending. If the
> > > > server is really running malicious code, then the queue_depth is the
> > > > last thing that the client needs to worry about.
> > >
> > > Like what? for an example?
> >
> > Like accessing compromised block devices. If the queue_depth is
> > garbage, the client would fail at allocation with ENOMEM; thats it.
>
> The client will get wrong data, check it and discard. The case of ENOMEM triggered
> by remote side is different. It can cause to DDOS on the client.

True. This makes sense. Will drop this patch.

Thanks.

>
> Thanks
>
> >
> > >
> > > Thanks

[PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Gioh Kim]

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

It was difficult to find out why it failed to establish RDMA
connection. This patch adds some messages to show which function
has failed why.

Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 3d09d01e34b4..df17dd4c1e28 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
 	 * If this request is not the first connection request from the
 	 * client for this session then fail and return error.
 	 */
-	if (!first_conn)
+	if (!first_conn) {
+		pr_err("Error: Not the first connection request for this session\n");
 		return ERR_PTR(-ENXIO);
+	}
 
 	/* need to allocate a new srv */
 	srv = kzalloc(sizeof(*srv), GFP_KERNEL);
@@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 	srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
 	if (IS_ERR(srv)) {
 		err = PTR_ERR(srv);
+		pr_err("get_or_create_srv(), error %d\n", err);
 		goto reject_w_err;
 	}
 	mutex_lock(&srv->paths_mutex);
@@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 			mutex_unlock(&srv->paths_mutex);
 			put_srv(srv);
 			err = PTR_ERR(sess);
+			pr_err("RTRS server session allocation failed: %d\n", err);
 			goto reject_w_err;
 		}
 	}
 	err = create_con(sess, cm_id, cid);
 	if (err) {
+		rtrs_err((&sess->s), "create_con(), error %d\n", err);
 		(void)rtrs_rdma_do_reject(cm_id, err);
 		/*
 		 * Since session has other connections we follow normal way
@@ -1865,6 +1870,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 	}
 	err = rtrs_rdma_do_accept(sess, cm_id);
 	if (err) {
+		rtrs_err((&sess->s), "rtrs_rdma_do_accept(), error %d\n", err);
 		(void)rtrs_rdma_do_reject(cm_id, err);
 		/*
 		 * Since current connection was successfully added to the
-- 
2.25.1

Re: [PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Leon Romanovsky]

On Mon, May 03, 2021 at 01:48:02PM +0200, Gioh Kim wrote:
> From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> 
> It was difficult to find out why it failed to establish RDMA
> connection. This patch adds some messages to show which function
> has failed why.
> 
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 3d09d01e34b4..df17dd4c1e28 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
>  	 * If this request is not the first connection request from the
>  	 * client for this session then fail and return error.
>  	 */
> -	if (!first_conn)
> +	if (!first_conn) {
> +		pr_err("Error: Not the first connection request for this session\n");
>  		return ERR_PTR(-ENXIO);
> +	}
>  
>  	/* need to allocate a new srv */
>  	srv = kzalloc(sizeof(*srv), GFP_KERNEL);
> @@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>  	srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
>  	if (IS_ERR(srv)) {
>  		err = PTR_ERR(srv);
> +		pr_err("get_or_create_srv(), error %d\n", err);
>  		goto reject_w_err;
>  	}
>  	mutex_lock(&srv->paths_mutex);
> @@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>  			mutex_unlock(&srv->paths_mutex);
>  			put_srv(srv);
>  			err = PTR_ERR(sess);
> +			pr_err("RTRS server session allocation failed: %d\n", err);
>  			goto reject_w_err;
>  		}
>  	}
>  	err = create_con(sess, cm_id, cid);
>  	if (err) {
> +		rtrs_err((&sess->s), "create_con(), error %d\n", err);
>  		(void)rtrs_rdma_do_reject(cm_id, err);

Unrelated to this change, but this (void) casting should be go.

Thanks

>  		/*
>  		 * Since session has other connections we follow normal way
> @@ -1865,6 +1870,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>  	}
>  	err = rtrs_rdma_do_accept(sess, cm_id);
>  	if (err) {
> +		rtrs_err((&sess->s), "rtrs_rdma_do_accept(), error %d\n", err);
>  		(void)rtrs_rdma_do_reject(cm_id, err);
>  		/*
>  		 * Since current connection was successfully added to the
> -- 
> 2.25.1
> 

Re: [PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Haris Iqbal]

On Sun, May 9, 2021 at 1:27 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 03, 2021 at 01:48:02PM +0200, Gioh Kim wrote:
> > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> >
> > It was difficult to find out why it failed to establish RDMA
> > connection. This patch adds some messages to show which function
> > has failed why.
> >
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > ---
> >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > index 3d09d01e34b4..df17dd4c1e28 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> >        * If this request is not the first connection request from the
> >        * client for this session then fail and return error.
> >        */
> > -     if (!first_conn)
> > +     if (!first_conn) {
> > +             pr_err("Error: Not the first connection request for this session\n");
> >               return ERR_PTR(-ENXIO);
> > +     }
> >
> >       /* need to allocate a new srv */
> >       srv = kzalloc(sizeof(*srv), GFP_KERNEL);
> > @@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >       srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> >       if (IS_ERR(srv)) {
> >               err = PTR_ERR(srv);
> > +             pr_err("get_or_create_srv(), error %d\n", err);
> >               goto reject_w_err;
> >       }
> >       mutex_lock(&srv->paths_mutex);
> > @@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >                       mutex_unlock(&srv->paths_mutex);
> >                       put_srv(srv);
> >                       err = PTR_ERR(sess);
> > +                     pr_err("RTRS server session allocation failed: %d\n", err);
> >                       goto reject_w_err;
> >               }
> >       }
> >       err = create_con(sess, cm_id, cid);
> >       if (err) {
> > +             rtrs_err((&sess->s), "create_con(), error %d\n", err);
> >               (void)rtrs_rdma_do_reject(cm_id, err);
>
> Unrelated to this change, but this (void) casting should be go.

Hi Leon

We wanted to explicitly signal that the code is ignoring the return
value of the function. Is there a strong reason for the casting to be
removed?

>
> Thanks
>
> >               /*
> >                * Since session has other connections we follow normal way
> > @@ -1865,6 +1870,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >       }
> >       err = rtrs_rdma_do_accept(sess, cm_id);
> >       if (err) {
> > +             rtrs_err((&sess->s), "rtrs_rdma_do_accept(), error %d\n", err);
> >               (void)rtrs_rdma_do_reject(cm_id, err);
> >               /*
> >                * Since current connection was successfully added to the
> > --
> > 2.25.1
> >

Re: [PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Leon Romanovsky]

On Mon, May 10, 2021 at 12:55:42PM +0200, Haris Iqbal wrote:
> On Sun, May 9, 2021 at 1:27 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Mon, May 03, 2021 at 01:48:02PM +0200, Gioh Kim wrote:
> > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > >
> > > It was difficult to find out why it failed to establish RDMA
> > > connection. This patch adds some messages to show which function
> > > has failed why.
> > >
> > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > ---
> > >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > index 3d09d01e34b4..df17dd4c1e28 100644
> > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > >        * If this request is not the first connection request from the
> > >        * client for this session then fail and return error.
> > >        */
> > > -     if (!first_conn)
> > > +     if (!first_conn) {
> > > +             pr_err("Error: Not the first connection request for this session\n");
> > >               return ERR_PTR(-ENXIO);
> > > +     }
> > >
> > >       /* need to allocate a new srv */
> > >       srv = kzalloc(sizeof(*srv), GFP_KERNEL);
> > > @@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > >       srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> > >       if (IS_ERR(srv)) {
> > >               err = PTR_ERR(srv);
> > > +             pr_err("get_or_create_srv(), error %d\n", err);
> > >               goto reject_w_err;
> > >       }
> > >       mutex_lock(&srv->paths_mutex);
> > > @@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > >                       mutex_unlock(&srv->paths_mutex);
> > >                       put_srv(srv);
> > >                       err = PTR_ERR(sess);
> > > +                     pr_err("RTRS server session allocation failed: %d\n", err);
> > >                       goto reject_w_err;
> > >               }
> > >       }
> > >       err = create_con(sess, cm_id, cid);
> > >       if (err) {
> > > +             rtrs_err((&sess->s), "create_con(), error %d\n", err);
> > >               (void)rtrs_rdma_do_reject(cm_id, err);
> >
> > Unrelated to this change, but this (void) casting should be go.
> 
> Hi Leon
> 
> We wanted to explicitly signal that the code is ignoring the return
> value of the function. Is there a strong reason for the casting to be
> removed?

"Don't write useless code and don't assume that the kernel developers
are first year college students" - is this strong enough reason for you?

Your (void) casting doesn't give anything extra, just churn and even
more suspicious review over such code.

Thanks

Re: [PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Haris Iqbal]

On Mon, May 10, 2021 at 2:03 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 10, 2021 at 12:55:42PM +0200, Haris Iqbal wrote:
> > On Sun, May 9, 2021 at 1:27 PM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Mon, May 03, 2021 at 01:48:02PM +0200, Gioh Kim wrote:
> > > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > > >
> > > > It was difficult to find out why it failed to establish RDMA
> > > > connection. This patch adds some messages to show which function
> > > > has failed why.
> > > >
> > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > > ---
> > > >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > index 3d09d01e34b4..df17dd4c1e28 100644
> > > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > > >        * If this request is not the first connection request from the
> > > >        * client for this session then fail and return error.
> > > >        */
> > > > -     if (!first_conn)
> > > > +     if (!first_conn) {
> > > > +             pr_err("Error: Not the first connection request for this session\n");
> > > >               return ERR_PTR(-ENXIO);
> > > > +     }
> > > >
> > > >       /* need to allocate a new srv */
> > > >       srv = kzalloc(sizeof(*srv), GFP_KERNEL);
> > > > @@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > > >       srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> > > >       if (IS_ERR(srv)) {
> > > >               err = PTR_ERR(srv);
> > > > +             pr_err("get_or_create_srv(), error %d\n", err);
> > > >               goto reject_w_err;
> > > >       }
> > > >       mutex_lock(&srv->paths_mutex);
> > > > @@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > > >                       mutex_unlock(&srv->paths_mutex);
> > > >                       put_srv(srv);
> > > >                       err = PTR_ERR(sess);
> > > > +                     pr_err("RTRS server session allocation failed: %d\n", err);
> > > >                       goto reject_w_err;
> > > >               }
> > > >       }
> > > >       err = create_con(sess, cm_id, cid);
> > > >       if (err) {
> > > > +             rtrs_err((&sess->s), "create_con(), error %d\n", err);
> > > >               (void)rtrs_rdma_do_reject(cm_id, err);
> > >
> > > Unrelated to this change, but this (void) casting should be go.
> >
> > Hi Leon
> >
> > We wanted to explicitly signal that the code is ignoring the return
> > value of the function. Is there a strong reason for the casting to be
> > removed?
>
> "Don't write useless code and don't assume that the kernel developers
> are first year college students" - is this strong enough reason for you?

I am all for simple and minimum code; but this casting does have a
decent reason.

Anyway if the RDMA code style calls for this to be removed; will
remove them in a separate patch and send out with the next batch of
patches.

>
> Your (void) casting doesn't give anything extra, just churn and even
> more suspicious review over such code.
>
> Thanks

Re: [PATCH for-next 04/20] RDMA/rtrs-srv: Add error messages for cases when failing RDMA connection

[Leon Romanovsky]

On Mon, May 10, 2021 at 02:16:02PM +0200, Haris Iqbal wrote:
> On Mon, May 10, 2021 at 2:03 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Mon, May 10, 2021 at 12:55:42PM +0200, Haris Iqbal wrote:
> > > On Sun, May 9, 2021 at 1:27 PM Leon Romanovsky <leon@kernel.org> wrote:
> > > >
> > > > On Mon, May 03, 2021 at 01:48:02PM +0200, Gioh Kim wrote:
> > > > > From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>
> > > > >
> > > > > It was difficult to find out why it failed to establish RDMA
> > > > > connection. This patch adds some messages to show which function
> > > > > has failed why.
> > > > >
> > > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > > > Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> > > > > ---
> > > > >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 8 +++++++-
> > > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > > index 3d09d01e34b4..df17dd4c1e28 100644
> > > > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > > > @@ -1356,8 +1356,10 @@ static struct rtrs_srv *get_or_create_srv(struct rtrs_srv_ctx *ctx,
> > > > >        * If this request is not the first connection request from the
> > > > >        * client for this session then fail and return error.
> > > > >        */
> > > > > -     if (!first_conn)
> > > > > +     if (!first_conn) {
> > > > > +             pr_err("Error: Not the first connection request for this session\n");
> > > > >               return ERR_PTR(-ENXIO);
> > > > > +     }
> > > > >
> > > > >       /* need to allocate a new srv */
> > > > >       srv = kzalloc(sizeof(*srv), GFP_KERNEL);
> > > > > @@ -1812,6 +1814,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > > > >       srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> > > > >       if (IS_ERR(srv)) {
> > > > >               err = PTR_ERR(srv);
> > > > > +             pr_err("get_or_create_srv(), error %d\n", err);
> > > > >               goto reject_w_err;
> > > > >       }
> > > > >       mutex_lock(&srv->paths_mutex);
> > > > > @@ -1850,11 +1853,13 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > > > >                       mutex_unlock(&srv->paths_mutex);
> > > > >                       put_srv(srv);
> > > > >                       err = PTR_ERR(sess);
> > > > > +                     pr_err("RTRS server session allocation failed: %d\n", err);
> > > > >                       goto reject_w_err;
> > > > >               }
> > > > >       }
> > > > >       err = create_con(sess, cm_id, cid);
> > > > >       if (err) {
> > > > > +             rtrs_err((&sess->s), "create_con(), error %d\n", err);
> > > > >               (void)rtrs_rdma_do_reject(cm_id, err);
> > > >
> > > > Unrelated to this change, but this (void) casting should be go.
> > >
> > > Hi Leon
> > >
> > > We wanted to explicitly signal that the code is ignoring the return
> > > value of the function. Is there a strong reason for the casting to be
> > > removed?
> >
> > "Don't write useless code and don't assume that the kernel developers
> > are first year college students" - is this strong enough reason for you?
> 
> I am all for simple and minimum code; but this casting does have a
> decent reason.
> 
> Anyway if the RDMA code style calls for this to be removed; will
> remove them in a separate patch and send out with the next batch of
> patches.

Yes, please, we prefer proper APIs that don't require any special
comments to use them.

Thanks

> 
> >
> > Your (void) casting doesn't give anything extra, just churn and even
> > more suspicious review over such code.
> >
> > Thanks

[PATCH for-next 05/20] RDMA/rtrs-srv: Clean up the code in __rtrs_srv_change_state

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

No need to use double switch to check the change of state everywhere,
let's change them to "if" to reduce size.

Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index df17dd4c1e28..74a5dfe85813 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -77,32 +77,17 @@ static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
 	old_state = sess->state;
 	switch (new_state) {
 	case RTRS_SRV_CONNECTED:
-		switch (old_state) {
-		case RTRS_SRV_CONNECTING:
+		if (old_state == RTRS_SRV_CONNECTING)
 			changed = true;
-			fallthrough;
-		default:
-			break;
-		}
 		break;
 	case RTRS_SRV_CLOSING:
-		switch (old_state) {
-		case RTRS_SRV_CONNECTING:
-		case RTRS_SRV_CONNECTED:
+		if (old_state == RTRS_SRV_CONNECTING ||
+		    old_state == RTRS_SRV_CONNECTED)
 			changed = true;
-			fallthrough;
-		default:
-			break;
-		}
 		break;
 	case RTRS_SRV_CLOSED:
-		switch (old_state) {
-		case RTRS_SRV_CLOSING:
+		if (old_state == RTRS_SRV_CLOSING)
 			changed = true;
-			fallthrough;
-		default:
-			break;
-		}
 		break;
 	default:
 		break;
-- 
2.25.1

[PATCH for-next 06/20] RDMA/rtrs: Change MAX_SESS_QUEUE_DEPTH

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

Max IB immediate data size is 2^28 (MAX_IMM_PAYL_BITS)
and the minimum chunk size is 4096 (2^12).
Therefore the maximum sess_queue_depth is 65536 (2^16).

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-pri.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index bcad5e2168c5..62924ad5362d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -47,12 +47,15 @@ enum {
 	MAX_PATHS_NUM = 128,
 
 	/*
-	 * With the size of struct rtrs_permit allocated on the client, 4K
-	 * is the maximum number of rtrs_permits we can allocate. This number is
-	 * also used on the client to allocate the IU for the user connection
-	 * to receive the RDMA addresses from the server.
+	 * Max IB immediate data size is 2^28 (MAX_IMM_PAYL_BITS)
+	 * and the minimum chunk size is 4096 (2^12).
+	 * So the maximum sess_queue_depth is 65536 (2^16) in theory.
+	 * But mempool_create, create_qp and ib_post_send fail with
+	 * "cannot allocate memory" error if sess_queue_depth is too big.
+	 * Therefore the pratical max value of sess_queue_depth is
+	 * somewhere between 1 and 65536 and it depends on the system.
 	 */
-	MAX_SESS_QUEUE_DEPTH = 4096,
+	MAX_SESS_QUEUE_DEPTH = 65536,
 
 	RTRS_HB_INTERVAL_MS = 5000,
 	RTRS_HB_MISSED_MAX = 5,
-- 
2.25.1

[PATCH for-next 07/20] RDMA/rtrs: Define MIN_CHUNK_SIZE

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

Define MIN_CHUNK_SIZE to replace the hard-coding number.
We need 4k for metadata, so MIN_CHUNK_SIZE should be at least 8k.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index 62924ad5362d..a092f7d7c7b3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -56,6 +56,7 @@ enum {
 	 * somewhere between 1 and 65536 and it depends on the system.
 	 */
 	MAX_SESS_QUEUE_DEPTH = 65536,
+	MIN_CHUNK_SIZE = 8192,
 
 	RTRS_HB_INTERVAL_MS = 5000,
 	RTRS_HB_MISSED_MAX = 5,
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 74a5dfe85813..02bc704667a8 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -2165,9 +2165,9 @@ static int check_module_params(void)
 		       sess_queue_depth, 1, MAX_SESS_QUEUE_DEPTH);
 		return -EINVAL;
 	}
-	if (max_chunk_size < 4096 || !is_power_of_2(max_chunk_size)) {
+	if (max_chunk_size < MIN_CHUNK_SIZE || !is_power_of_2(max_chunk_size)) {
 		pr_err("Invalid max_chunk_size value %d, has to be >= %d and should be power of two.\n",
-		       max_chunk_size, 4096);
+		       max_chunk_size, MIN_CHUNK_SIZE);
 		return -EINVAL;
 	}
 
-- 
2.25.1

[PATCH for-next 08/20] RDMA/rtrs: Use strscpy instead of strlcpy

[Gioh Kim]

From: Dima Stepanov <dmitrii.stepanov@cloud.ionos.com>

During checkpatch analyzing the following warning message was found:
  WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
  https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Fix it by using strscpy calls instead of strlcpy.

Signed-off-by: Dima Stepanov <dmitrii.stepanov@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 4 ++--
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 0c828ea0f500..5ba87862a251 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1499,7 +1499,7 @@ static struct rtrs_clt_sess *alloc_sess(struct rtrs_clt *clt,
 	if (path->src)
 		memcpy(&sess->s.src_addr, path->src,
 		       rdma_addr_size((struct sockaddr *)path->src));
-	strlcpy(sess->s.sessname, clt->sessname, sizeof(sess->s.sessname));
+	strscpy(sess->s.sessname, clt->sessname, sizeof(sess->s.sessname));
 	sess->s.con_num = con_num;
 	sess->clt = clt;
 	sess->max_pages_per_mr = max_segments * max_segment_size >> 12;
@@ -2638,7 +2638,7 @@ static struct rtrs_clt *alloc_clt(const char *sessname, size_t paths_num,
 	clt->priv = priv;
 	clt->link_ev = link_ev;
 	clt->mp_policy = MP_POLICY_MIN_INFLIGHT;
-	strlcpy(clt->sessname, sessname, sizeof(clt->sessname));
+	strscpy(clt->sessname, sessname, sizeof(clt->sessname));
 	init_waitqueue_head(&clt->permits_wait);
 	mutex_init(&clt->paths_ev_mutex);
 	mutex_init(&clt->paths_mutex);
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 02bc704667a8..afa63f06586b 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -780,7 +780,7 @@ static int process_info_req(struct rtrs_srv_con *con,
 	rwr = kcalloc(sess->mrs_num, sizeof(*rwr), GFP_KERNEL);
 	if (unlikely(!rwr))
 		return -ENOMEM;
-	strlcpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
+	strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
 
 	tx_sz  = sizeof(*rsp);
 	tx_sz += sizeof(rsp->desc[0]) * sess->mrs_num;
@@ -1261,7 +1261,7 @@ int rtrs_srv_get_sess_name(struct rtrs_srv *srv, char *sessname, size_t len)
 	list_for_each_entry(sess, &srv->paths_list, s.entry) {
 		if (sess->state != RTRS_SRV_CONNECTED)
 			continue;
-		strlcpy(sessname, sess->s.sessname,
+		strscpy(sessname, sess->s.sessname,
 		       min_t(size_t, sizeof(sess->s.sessname), len));
 		err = 0;
 		break;
@@ -1715,7 +1715,7 @@ static struct rtrs_srv_sess *__alloc_sess(struct rtrs_srv *srv,
 	path.src = &sess->s.src_addr;
 	path.dst = &sess->s.dst_addr;
 	rtrs_addr_to_str(&path, str, sizeof(str));
-	strlcpy(sess->s.sessname, str, sizeof(sess->s.sessname));
+	strscpy(sess->s.sessname, str, sizeof(sess->s.sessname));
 
 	sess->s.con_num = con_num;
 	sess->s.recon_cnt = recon_cnt;
-- 
2.25.1

[PATCH for-next 09/20] RDMA/rtrs-clt: Kill rtrs_clt_{start,stop}_hb

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

The two wrappers are not needed since we can call rtrs_{start,stop}_hb
directly.

Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 5ba87862a251..76040f6c43b2 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1449,16 +1449,6 @@ static void rtrs_clt_init_hb(struct rtrs_clt_sess *sess)
 		      rtrs_wq);
 }
 
-static void rtrs_clt_start_hb(struct rtrs_clt_sess *sess)
-{
-	rtrs_start_hb(&sess->s);
-}
-
-static void rtrs_clt_stop_hb(struct rtrs_clt_sess *sess)
-{
-	rtrs_stop_hb(&sess->s);
-}
-
 static void rtrs_clt_reconnect_work(struct work_struct *work);
 static void rtrs_clt_close_work(struct work_struct *work);
 
@@ -2082,7 +2072,7 @@ static void rtrs_clt_stop_and_destroy_conns(struct rtrs_clt_sess *sess)
 	 */
 	synchronize_rcu();
 
-	rtrs_clt_stop_hb(sess);
+	rtrs_stop_hb(&sess->s);
 
 	/*
 	 * The order it utterly crucial: firstly disconnect and complete all
@@ -2275,7 +2265,7 @@ static int init_conns(struct rtrs_clt_sess *sess)
 	if (err)
 		goto destroy;
 
-	rtrs_clt_start_hb(sess);
+	rtrs_start_hb(&sess->s);
 
 	return 0;
 
-- 
2.25.1

[PATCH for-next 10/20] RDMA/rtrs-clt: Kill rtrs_clt_disconnect_from_sysfs

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

The function is just a wrapper of rtrs_clt_close_conns, let's call
rtrs_clt_close_conns directly.

Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c | 5 +----
 drivers/infiniband/ulp/rtrs/rtrs-clt.c       | 9 +--------
 drivers/infiniband/ulp/rtrs/rtrs-clt.h       | 2 +-
 3 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
index 7d53d18a5004..4ee592ccf979 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c
@@ -250,7 +250,6 @@ static ssize_t rtrs_clt_disconnect_store(struct kobject *kobj,
 					  const char *buf, size_t count)
 {
 	struct rtrs_clt_sess *sess;
-	int ret;
 
 	sess = container_of(kobj, struct rtrs_clt_sess, kobj);
 	if (!sysfs_streq(buf, "1")) {
@@ -258,9 +257,7 @@ static ssize_t rtrs_clt_disconnect_store(struct kobject *kobj,
 			  attr->attr.name, buf);
 		return -EINVAL;
 	}
-	ret = rtrs_clt_disconnect_from_sysfs(sess);
-	if (ret)
-		return ret;
+	rtrs_clt_close_conns(sess, true);
 
 	return count;
 }
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 76040f6c43b2..d9764d1fe0f4 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1843,7 +1843,7 @@ static int rtrs_rdma_conn_rejected(struct rtrs_clt_con *con,
 	return -ECONNRESET;
 }
 
-static void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait)
+void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait)
 {
 	if (rtrs_clt_change_state_get_old(sess, RTRS_CLT_CLOSING, NULL))
 		queue_work(rtrs_wq, &sess->close_work);
@@ -2818,13 +2818,6 @@ int rtrs_clt_reconnect_from_sysfs(struct rtrs_clt_sess *sess)
 	return err;
 }
 
-int rtrs_clt_disconnect_from_sysfs(struct rtrs_clt_sess *sess)
-{
-	rtrs_clt_close_conns(sess, true);
-
-	return 0;
-}
-
 int rtrs_clt_remove_path_from_sysfs(struct rtrs_clt_sess *sess,
 				     const struct attribute *sysfs_self)
 {
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.h b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
index ef15927dfeda..0a0ea43d2b46 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.h
@@ -203,7 +203,7 @@ static inline struct rtrs_permit *get_permit(struct rtrs_clt *clt, int idx)
 }
 
 int rtrs_clt_reconnect_from_sysfs(struct rtrs_clt_sess *sess);
-int rtrs_clt_disconnect_from_sysfs(struct rtrs_clt_sess *sess);
+void rtrs_clt_close_conns(struct rtrs_clt_sess *sess, bool wait);
 int rtrs_clt_create_path_from_sysfs(struct rtrs_clt *clt,
 				     struct rtrs_addr *addr);
 int rtrs_clt_remove_path_from_sysfs(struct rtrs_clt_sess *sess,
-- 
2.25.1

[PATCH for-next 11/20] RDMA/rtrs-srv: Kill __rtrs_srv_change_state

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

No need since the only user is rtrs_srv_change_state.

Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index afa63f06586b..54a29285240d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -67,13 +67,13 @@ static inline struct rtrs_srv_sess *to_srv_sess(struct rtrs_sess *s)
 	return container_of(s, struct rtrs_srv_sess, s);
 }
 
-static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
-				     enum rtrs_srv_state new_state)
+static bool rtrs_srv_change_state(struct rtrs_srv_sess *sess,
+				  enum rtrs_srv_state new_state)
 {
 	enum rtrs_srv_state old_state;
 	bool changed = false;
 
-	lockdep_assert_held(&sess->state_lock);
+	spin_lock_irq(&sess->state_lock);
 	old_state = sess->state;
 	switch (new_state) {
 	case RTRS_SRV_CONNECTED:
@@ -94,17 +94,6 @@ static bool __rtrs_srv_change_state(struct rtrs_srv_sess *sess,
 	}
 	if (changed)
 		sess->state = new_state;
-
-	return changed;
-}
-
-static bool rtrs_srv_change_state(struct rtrs_srv_sess *sess,
-				   enum rtrs_srv_state new_state)
-{
-	bool changed;
-
-	spin_lock_irq(&sess->state_lock);
-	changed = __rtrs_srv_change_state(sess, new_state);
 	spin_unlock_irq(&sess->state_lock);
 
 	return changed;
-- 
2.25.1

[PATCH for-next 12/20] RDMA/rtrs-clt: Remove redundant 'break'

[Gioh Kim]

From: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>

It is duplicated with the very next line

Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Reviewed-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index d9764d1fe0f4..a57b77998573 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -655,7 +655,6 @@ static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
 			rtrs_err(con->c.sess, "rtrs_post_recv_empty(): %d\n",
 				  err);
 			rtrs_rdma_error_recovery(con);
-			break;
 		}
 		break;
 	case IB_WC_RECV:
-- 
2.25.1

[PATCH for-next 13/20] RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading its stats

[Gioh Kim]

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

When get_next_path_min_inflight is called to select the next path, it
iterates over the list of available rtrs_clt_sess (paths). It then reads
the number of inflight IOs for that path to select one which has the least
inflight IO.

But it may so happen that rtrs_clt_sess (path) is no longer in the
connected state because closing or error recovery paths can change
the status of the rtrs_clt_Sess.

For example, the client sent the heart-beat and did not get the
response, it would change the session status and stop IO processing.
The added checking of this patch can prevent accessing the broken path
and generating duplicated error messages.

It is ok if the status is changed after checking the status because
the error recovery path does not free memory and only tries to
reconnection. And also it is ok if the session is closed after checking
the status because closing the session changes the session status and
flush all IO beforing free memory. If the session is being accessed for
IO processing, the closing session will wait.

Fixes: 6a98d71daea18 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index a57b77998573..c8a7807793bf 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -813,6 +813,9 @@ static struct rtrs_clt_sess *get_next_path_min_inflight(struct path_it *it)
 	int inflight;
 
 	list_for_each_entry_rcu(sess, &clt->paths_list, s.entry) {
+		if (unlikely(READ_ONCE(sess->state) != RTRS_CLT_CONNECTED))
+			continue;
+
 		if (unlikely(!list_empty(raw_cpu_ptr(sess->mp_skip_entry))))
 			continue;
 
-- 
2.25.1

[PATCH for-next 14/20] RDMA/rtrs-srv: Replace atomic_t with percpu_ref for ids_inflight

[Gioh Kim]

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

ids_inflight is used to track the inflight IOs. But the use of atomic_t
variable can cause performance drops and can also become a performance
bottleneck.

This commit replaces the use of atomic_t with a percpu_ref structure. The
advantage it offers is, it doesn't check if the reference has fallen to 0,
until the user explicitly signals it to; and that is done by the
percpu_ref_kill() function call. After that, the percpu_ref structure
behaves like an atomic_t and for every put call, checks whether the
reference has fallen to 0 or not.

rtrs_srv_stats_rdma_to_str shows the count of ids_inflight as 0
for user-mode tools not to be confused.

Fixes: 9cb837480424e ("RDMA/rtrs: server: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c | 12 +++---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c       | 43 +++++++++++++-------
 drivers/infiniband/ulp/rtrs/rtrs-srv.h       |  4 +-
 3 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
index e102b1368d0c..df1d7d6b1884 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-stats.c
@@ -27,12 +27,10 @@ ssize_t rtrs_srv_stats_rdma_to_str(struct rtrs_srv_stats *stats,
 				    char *page, size_t len)
 {
 	struct rtrs_srv_stats_rdma_stats *r = &stats->rdma_stats;
-	struct rtrs_srv_sess *sess = stats->sess;
 
-	return scnprintf(page, len, "%lld %lld %lld %lld %u\n",
-			 (s64)atomic64_read(&r->dir[READ].cnt),
-			 (s64)atomic64_read(&r->dir[READ].size_total),
-			 (s64)atomic64_read(&r->dir[WRITE].cnt),
-			 (s64)atomic64_read(&r->dir[WRITE].size_total),
-			 atomic_read(&sess->ids_inflight));
+	return sysfs_emit(page, "%lld %lld %lld %lldn\n",
+			  (s64)atomic64_read(&r->dir[READ].cnt),
+			  (s64)atomic64_read(&r->dir[READ].size_total),
+			  (s64)atomic64_read(&r->dir[WRITE].cnt),
+			  (s64)atomic64_read(&r->dir[WRITE].size_total));
 }
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 54a29285240d..f66f2be9f519 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -111,7 +111,6 @@ static void rtrs_srv_free_ops_ids(struct rtrs_srv_sess *sess)
 	struct rtrs_srv *srv = sess->srv;
 	int i;
 
-	WARN_ON(atomic_read(&sess->ids_inflight));
 	if (sess->ops_ids) {
 		for (i = 0; i < srv->queue_depth; i++)
 			free_id(sess->ops_ids[i]);
@@ -126,11 +125,19 @@ static struct ib_cqe io_comp_cqe = {
 	.done = rtrs_srv_rdma_done
 };
 
+static inline void rtrs_srv_inflight_ref_release(struct percpu_ref *ref)
+{
+	struct rtrs_srv_sess *sess = container_of(ref, struct rtrs_srv_sess, ids_inflight_ref);
+
+	percpu_ref_exit(&sess->ids_inflight_ref);
+	complete(&sess->complete_done);
+}
+
 static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
 {
 	struct rtrs_srv *srv = sess->srv;
 	struct rtrs_srv_op *id;
-	int i;
+	int i, ret;
 
 	sess->ops_ids = kcalloc(srv->queue_depth, sizeof(*sess->ops_ids),
 				GFP_KERNEL);
@@ -144,8 +151,14 @@ static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
 
 		sess->ops_ids[i] = id;
 	}
-	init_waitqueue_head(&sess->ids_waitq);
-	atomic_set(&sess->ids_inflight, 0);
+
+	ret = percpu_ref_init(&sess->ids_inflight_ref,
+			      rtrs_srv_inflight_ref_release, 0, GFP_KERNEL);
+	if (ret) {
+		pr_err("Percpu reference init failed\n");
+		goto err;
+	}
+	init_completion(&sess->complete_done);
 
 	return 0;
 
@@ -156,21 +169,14 @@ static int rtrs_srv_alloc_ops_ids(struct rtrs_srv_sess *sess)
 
 static inline void rtrs_srv_get_ops_ids(struct rtrs_srv_sess *sess)
 {
-	atomic_inc(&sess->ids_inflight);
+	percpu_ref_get(&sess->ids_inflight_ref);
 }
 
 static inline void rtrs_srv_put_ops_ids(struct rtrs_srv_sess *sess)
 {
-	if (atomic_dec_and_test(&sess->ids_inflight))
-		wake_up(&sess->ids_waitq);
+	percpu_ref_put(&sess->ids_inflight_ref);
 }
 
-static void rtrs_srv_wait_ops_ids(struct rtrs_srv_sess *sess)
-{
-	wait_event(sess->ids_waitq, !atomic_read(&sess->ids_inflight));
-}
-
-
 static void rtrs_srv_reg_mr_done(struct ib_cq *cq, struct ib_wc *wc)
 {
 	struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
@@ -1479,8 +1485,15 @@ static void rtrs_srv_close_work(struct work_struct *work)
 		rdma_disconnect(con->c.cm_id);
 		ib_drain_qp(con->c.qp);
 	}
-	/* Wait for all inflights */
-	rtrs_srv_wait_ops_ids(sess);
+
+	/*
+	 * Degrade ref count to the usual model with a single shared
+	 * atomic_t counter
+	 */
+	percpu_ref_kill(&sess->ids_inflight_ref);
+
+	/* Wait for all completion */
+	wait_for_completion(&sess->complete_done);
 
 	/* Notify upper layer if we are the last path */
 	rtrs_srv_sess_down(sess);
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.h b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
index 9543ae19996c..f8da2e3f0bda 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
@@ -81,8 +81,8 @@ struct rtrs_srv_sess {
 	spinlock_t		state_lock;
 	int			cur_cq_vector;
 	struct rtrs_srv_op	**ops_ids;
-	atomic_t		ids_inflight;
-	wait_queue_head_t	ids_waitq;
+	struct percpu_ref       ids_inflight_ref;
+	struct completion       complete_done;
 	struct rtrs_srv_mr	*mrs;
 	unsigned int		mrs_num;
 	dma_addr_t		*dma_addr;
-- 
2.25.1

[PATCH for-next 15/20] RDMA/rtrs: Do not reset hb_missed_max after re-connection

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

When re-connecting, it resets hb_missed_max to 0.
Before the first re-connecting, client will trigger re-connection
when it gets hb-ack more than 5 times. But after the first
re-connecting, clients will do re-connection whenever it does
not get hb-ack because hb_missed_max is 0.

There is no need to reset hb_missed_max when re-connecting.
hb_missed_max should be kept until closing the session.

Fixes: c0894b3ea69d3 ("RDMA/rtrs: core: lib functions shared between client and server modules")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
index a7847282a2eb..4e602e40f623 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs.c
@@ -376,7 +376,6 @@ void rtrs_stop_hb(struct rtrs_sess *sess)
 {
 	cancel_delayed_work_sync(&sess->hb_dwork);
 	sess->hb_missed_cnt = 0;
-	sess->hb_missed_max = 0;
 }
 EXPORT_SYMBOL_GPL(rtrs_stop_hb);
 
-- 
2.25.1

[PATCH for-next 16/20] RDMA/rtrs-srv: Duplicated session name is not allowed

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

If two clients try to use the same session name, rtrs-server generates a
kernel error that it failed to create the sysfs because the filename
is duplicated.

This patch adds code to check if there already exists the same session
name with the different UUID. If a client tries to add more session,
it sends the UUID and the session name. Therefore it is ok if there is
already same session name with the same UUID. The rtrs-server must fail
only-if there is the same session name with the different UUID.

Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Aleksei Marov <aleksei.marov@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 42 +++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index f66f2be9f519..0e1e303dcbee 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -753,7 +753,40 @@ static void rtrs_srv_sess_down(struct rtrs_srv_sess *sess)
 	mutex_unlock(&srv->paths_ev_mutex);
 }
 
+static bool exist_sessname(struct rtrs_srv_ctx *ctx,
+			   const char *sessname, const uuid_t *path_uuid)
+{
+	struct rtrs_srv *srv;
+	struct rtrs_srv_sess *sess;
+	bool found = false;
+
+	mutex_lock(&ctx->srv_mutex);
+	list_for_each_entry(srv, &ctx->srv_list, ctx_list) {
+		mutex_lock(&srv->paths_mutex);
+
+		/* when a client with same uuid and same sessname tried to add a path */
+		if (uuid_equal(&srv->paths_uuid, path_uuid)) {
+			mutex_unlock(&srv->paths_mutex);
+			continue;
+		}
+
+		list_for_each_entry(sess, &srv->paths_list, s.entry) {
+			if (strlen(sess->s.sessname) == strlen(sessname) &&
+			    !strcmp(sess->s.sessname, sessname)) {
+				found = true;
+				break;
+			}
+		}
+		mutex_unlock(&srv->paths_mutex);
+		if (found)
+			break;
+	}
+	mutex_unlock(&ctx->srv_mutex);
+	return found;
+}
+
 static int post_recv_sess(struct rtrs_srv_sess *sess);
+static int rtrs_rdma_do_reject(struct rdma_cm_id *cm_id, int errno);
 
 static int process_info_req(struct rtrs_srv_con *con,
 			    struct rtrs_msg_info_req *msg)
@@ -772,10 +805,17 @@ static int process_info_req(struct rtrs_srv_con *con,
 		rtrs_err(s, "post_recv_sess(), err: %d\n", err);
 		return err;
 	}
+
+	if (exist_sessname(sess->srv->ctx,
+			   msg->sessname, &sess->srv->paths_uuid)) {
+		rtrs_err(s, "sessname is duplicated: %s\n", msg->sessname);
+		return -EPERM;
+	}
+	strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
+
 	rwr = kcalloc(sess->mrs_num, sizeof(*rwr), GFP_KERNEL);
 	if (unlikely(!rwr))
 		return -ENOMEM;
-	strscpy(sess->s.sessname, msg->sessname, sizeof(sess->s.sessname));
 
 	tx_sz  = sizeof(*rsp);
 	tx_sz += sizeof(rsp->desc[0]) * sess->mrs_num;
-- 
2.25.1

[PATCH for-next 17/20] RDMA/rtrs-srv: Fix memory leak of unfreed rtrs_srv_stats object

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

When closing a session, currently the rtrs_srv_stats object in the
closing session is freed by kobject release. But if it failed
to create a session by various reasons, it must free the rtrs_srv_stats
object directly because kobject is not created yet.

This problem is found by kmemleak as below:

1. One client machine maps /dev/nullb0 with session name 'bla':
root@test1:~# echo "sessname=bla path=ip:192.168.122.190 \
device_path=/dev/nullb0" > /sys/devices/virtual/rnbd-client/ctl/map_device

2. Another machine failed to create a session with the same name 'bla':
root@test2:~# echo "sessname=bla path=ip:192.168.122.190 \
device_path=/dev/nullb1" > /sys/devices/virtual/rnbd-client/ctl/map_device
-bash: echo: write error: Connection reset by peer

3. The kmemleak on server machine reported an error:
unreferenced object 0xffff888033cdc800 (size 128):
  comm "kworker/2:1", pid 83, jiffies 4295086585 (age 2508.680s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000a72903b2>] __alloc_sess+0x1d4/0x1250 [rtrs_server]
    [<00000000d1e5321e>] rtrs_srv_rdma_cm_handler+0xc31/0xde0 [rtrs_server]
    [<00000000bb2f6e7e>] cma_ib_req_handler+0xdc5/0x2b50 [rdma_cm]
    [<00000000e896235d>] cm_process_work+0x2d/0x100 [ib_cm]
    [<00000000b6866c5f>] cm_req_handler+0x11bc/0x1c40 [ib_cm]
    [<000000005f5dd9aa>] cm_work_handler+0xe65/0x3cf2 [ib_cm]
    [<00000000610151e7>] process_one_work+0x4bc/0x980
    [<00000000541e0f77>] worker_thread+0x78/0x5c0
    [<00000000423898ca>] kthread+0x191/0x1e0
    [<000000005a24b239>] ret_from_fork+0x3a/0x50

Fixes: 39c2d639ca183 ("RDMA/rtrs-srv: Set .release function for rtrs srv device during device init")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 0e1e303dcbee..879e6dbca86d 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1503,6 +1503,7 @@ static void free_sess(struct rtrs_srv_sess *sess)
 		kobject_del(&sess->kobj);
 		kobject_put(&sess->kobj);
 	} else {
+		kfree(sess->stats);
 		kfree(sess);
 	}
 }
-- 
2.25.1

[PATCH for-next 18/20] RDMA/rtrs-srv: Fix memory leak when having multiple sessions

[Gioh Kim]

From: Jack Wang <jinpu.wang@cloud.ionos.com>

Gioh notice memory leak below
unreferenced object 0xffff8880acda2000 (size 2048):
  comm "kworker/4:1", pid 77, jiffies 4295062871 (age 1270.730s)
  hex dump (first 32 bytes):
    00 20 da ac 80 88 ff ff 00 20 da ac 80 88 ff ff  . ....... ......
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000e85d85b5>] rtrs_srv_rdma_cm_handler+0x8e5/0xa90 [rtrs_server]
    [<00000000e31a988a>] cma_ib_req_handler+0xdc5/0x2b50 [rdma_cm]
    [<000000000eb02c5b>] cm_process_work+0x2d/0x100 [ib_cm]
    [<00000000e1650ca9>] cm_req_handler+0x11bc/0x1c40 [ib_cm]
    [<000000009c28818b>] cm_work_handler+0xe65/0x3cf2 [ib_cm]
    [<000000002b53eaa1>] process_one_work+0x4bc/0x980
    [<00000000da3499fb>] worker_thread+0x78/0x5c0
    [<00000000167127a4>] kthread+0x191/0x1e0
    [<0000000060802104>] ret_from_fork+0x3a/0x50
unreferenced object 0xffff88806d595d90 (size 8):
  comm "kworker/4:1H", pid 131, jiffies 4295062972 (age 1269.720s)
  hex dump (first 8 bytes):
    62 6c 61 00 6b 6b 6b a5                          bla.kkk.
  backtrace:
    [<000000004447d253>] kstrdup+0x2e/0x60
    [<0000000047259793>] kobject_set_name_vargs+0x2f/0xb0
    [<00000000c2ee3bc8>] dev_set_name+0xab/0xe0
    [<000000002b6bdfb1>] rtrs_srv_create_sess_files+0x260/0x290 [rtrs_server]
    [<0000000075d87bd7>] rtrs_srv_info_req_done+0x71b/0x960 [rtrs_server]
    [<00000000ccdf1bb5>] __ib_process_cq+0x94/0x100 [ib_core]
    [<00000000cbcb60cb>] ib_cq_poll_work+0x32/0xc0 [ib_core]
    [<000000002b53eaa1>] process_one_work+0x4bc/0x980
    [<00000000da3499fb>] worker_thread+0x78/0x5c0
    [<00000000167127a4>] kthread+0x191/0x1e0
    [<0000000060802104>] ret_from_fork+0x3a/0x50
unreferenced object 0xffff88806d6bb100 (size 256):
  comm "kworker/4:1H", pid 131, jiffies 4295062972 (age 1269.720s)
  hex dump (first 32 bytes):
    00 00 00 00 ad 4e ad de ff ff ff ff 00 00 00 00  .....N..........
    ff ff ff ff ff ff ff ff 00 59 4d 86 ff ff ff ff  .........YM.....
  backtrace:
    [<00000000a18a11e4>] device_add+0x74d/0xa00
    [<00000000a915b95f>] rtrs_srv_create_sess_files.cold+0x49/0x1fe [rtrs_server]
    [<0000000075d87bd7>] rtrs_srv_info_req_done+0x71b/0x960 [rtrs_server]
    [<00000000ccdf1bb5>] __ib_process_cq+0x94/0x100 [ib_core]
    [<00000000cbcb60cb>] ib_cq_poll_work+0x32/0xc0 [ib_core]
    [<000000002b53eaa1>] process_one_work+0x4bc/0x980
    [<00000000da3499fb>] worker_thread+0x78/0x5c0
    [<00000000167127a4>] kthread+0x191/0x1e0
    [<0000000060802104>] ret_from_fork+0x3a/0x50

The problem is we increase device refcount by get_device in
process_info_req for each path, but only does put_deice for last path,
which lead to memory leak.

To fix it, it also calls put_device when dev_ref is not 0.

Fixes: e2853c49477d1 ("RDMA/rtrs-srv-sysfs: fix missing put_device")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index a9288175fbb5..20efd44297fb 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -208,6 +208,7 @@ rtrs_srv_destroy_once_sysfs_root_folders(struct rtrs_srv_sess *sess)
 		device_del(&srv->dev);
 		put_device(&srv->dev);
 	} else {
+		put_device(&srv->dev);
 		mutex_unlock(&srv->paths_mutex);
 	}
 }
-- 
2.25.1

[PATCH for-next 19/20] RDMA/rtrs-clt: Check if the queue_depth has changed during a reconnection

[Gioh Kim]

From: Md Haris Iqbal <haris.iqbal@cloud.ionos.com>

The queue_depth is a module parameter for rtrs_server. It is used on the
client side to determing the queue_depth of the request queue for the RNBD
virtual block device.

During a reconnection event for an already mapped device, in case the
rtrs_server module queue_depth has changed, fail the reconnect attempt.

Also stop further auto reconnection attempts. A manual reconnect via
sysfs has to be triggerred.

Fixes: 6a98d71daea18 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index c8a7807793bf..ec7055b8136b 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1764,7 +1764,20 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
 	}
 	if (con->c.cid == 0) {
 		queue_depth = le16_to_cpu(msg->queue_depth);
-		if (!sess->rbufs || sess->queue_depth < queue_depth) {
+
+		if (sess->queue_depth > 0 && queue_depth != sess->queue_depth) {
+			rtrs_err(clt, "Error: queue depth changed\n");
+
+			/*
+			 * Stop any more reconnection attempts
+			 */
+			sess->reconnect_attempts = -1;
+			rtrs_err(clt,
+				"Disabling auto-reconnect. Trigger a manual reconnect after issue is resolved\n");
+			return -ECONNRESET;
+		}
+
+		if (!sess->rbufs) {
 			kfree(sess->rbufs);
 			sess->rbufs = kcalloc(queue_depth, sizeof(*sess->rbufs),
 					      GFP_KERNEL);
@@ -1778,7 +1791,7 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
 		sess->chunk_size = sess->max_io_size + sess->max_hdr_size;
 
 		/*
-		 * Global queue depth and IO size is always a minimum.
+		 * Global IO size is always a minimum.
 		 * If while a reconnection server sends us a value a bit
 		 * higher - client does not care and uses cached minimum.
 		 *
@@ -1786,8 +1799,7 @@ static int rtrs_rdma_conn_established(struct rtrs_clt_con *con,
 		 * connections in parallel, use lock.
 		 */
 		mutex_lock(&clt->paths_mutex);
-		clt->queue_depth = min_not_zero(sess->queue_depth,
-						clt->queue_depth);
+		clt->queue_depth = sess->queue_depth;
 		clt->max_io_size = min_not_zero(sess->max_io_size,
 						clt->max_io_size);
 		mutex_unlock(&clt->paths_mutex);
-- 
2.25.1

[PATCH for-next 20/20] RDMA/rtrs-clt: Fix memory leak of not-freed sess->stats and stats->pcpu_stats

[Gioh Kim]

From: Gioh Kim <gi-oh.kim@cloud.ionos.com>

sess->stats and sess->stats->pcpu_stats objects are freed
when sysfs entry is removed. If something wrong happens and
session is closed before sysfs entry is created,
sess->stats and sess->stats->pcpu_stats objects are not freed.

This patch adds freeing of them at three places:
1. When client uses wrong address and session creation fails.
2. When client fails to create a sysfs entry.
3. When client adds wrong address via sysfs add_path.

Fixes: 215378b838df0 ("RDMA/rtrs: client: sysfs interface functions")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index ec7055b8136b..136fb79355c7 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2753,6 +2753,8 @@ struct rtrs_clt *rtrs_clt_open(struct rtrs_clt_ops *ops,
 		if (err) {
 			list_del_rcu(&sess->s.entry);
 			rtrs_clt_close_conns(sess, true);
+			free_percpu(sess->stats->pcpu_stats);
+			kfree(sess->stats);
 			free_sess(sess);
 			goto close_all_sess;
 		}
@@ -2761,6 +2763,8 @@ struct rtrs_clt *rtrs_clt_open(struct rtrs_clt_ops *ops,
 		if (err) {
 			list_del_rcu(&sess->s.entry);
 			rtrs_clt_close_conns(sess, true);
+			free_percpu(sess->stats->pcpu_stats);
+			kfree(sess->stats);
 			free_sess(sess);
 			goto close_all_sess;
 		}
@@ -3011,6 +3015,8 @@ int rtrs_clt_create_path_from_sysfs(struct rtrs_clt *clt,
 close_sess:
 	rtrs_clt_remove_path_from_arr(sess);
 	rtrs_clt_close_conns(sess, true);
+	free_percpu(sess->stats->pcpu_stats);
+	kfree(sess->stats);
 	free_sess(sess);
 
 	return err;
-- 
2.25.1