diff mbox series

[RFC,08/12] RDMA/rtrs: Kill recon_cnt from several structs

Message ID 20221113010823.6436-9-guoqing.jiang@linux.dev (mailing list archive)
State Changes Requested
Headers show
Series Misc changes for rtrs | expand

Commit Message

Guoqing Jiang Nov. 13, 2022, 1:08 a.m. UTC
Seems the only relevant comment about recon_cnt is,

/*
 * On every new session connections increase reconnect counter
 * to avoid clashes with previous sessions not yet closed
 * sessions on a server side.
 */

However, it is not clear how the recon_cnt avoid clashed at these places
in the commit since no where checks it.

Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
---
 drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
 drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
 3 files changed, 1 insertion(+), 17 deletions(-)

Comments

Haris Iqbal Nov. 15, 2022, 9:39 a.m. UTC | #1
On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Seems the only relevant comment about recon_cnt is,
>
> /*
>  * On every new session connections increase reconnect counter
>  * to avoid clashes with previous sessions not yet closed
>  * sessions on a server side.
>  */
>
> However, it is not clear how the recon_cnt avoid clashed at these places
> in the commit since no where checks it.

It does seem redundant. This predates my time, so I don't know if
there was a change which removed the usage of this. I tried to search
in commit history, but couldn't.

@Jinpu Your thoughts?

>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
>  drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
>  3 files changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 5ffc170dae8c..dcc8c041a141 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -1802,7 +1802,6 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
>                 .version = cpu_to_le16(RTRS_PROTO_VER),
>                 .cid = cpu_to_le16(con->c.cid),
>                 .cid_num = cpu_to_le16(clt_path->s.con_num),
> -               .recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
>         };
>         msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
>         uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
> @@ -2336,13 +2335,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
>         unsigned int cid;
>         int err;
>
> -       /*
> -        * On every new session connections increase reconnect counter
> -        * to avoid clashes with previous sessions not yet closed
> -        * sessions on a server side.
> -        */
> -       clt_path->s.recon_cnt++;
> -
>         /* Establish all RDMA connections  */
>         for (cid = 0; cid < clt_path->s.con_num; cid++) {
>                 err = create_con(clt_path, cid);
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> index a2420eecaf5a..c4ddaeba1c59 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> @@ -109,7 +109,6 @@ struct rtrs_path {
>         struct rtrs_con **con;
>         unsigned int            con_num;
>         unsigned int            irq_con_num;
> -       unsigned int            recon_cnt;
>         unsigned int            signal_interval;
>         struct rtrs_ib_dev      *dev;
>         int                     dev_ref;
> @@ -177,7 +176,6 @@ struct rtrs_sg_desc {
>   * @version:      RTRS protocol version
>   * @cid:          Current connection id
>   * @cid_num:      Number of connections per session
> - * @recon_cnt:    Reconnections counter
>   * @sess_uuid:    UUID of a session (path)
>   * @paths_uuid:           UUID of a group of sessions (paths)
>   *
> @@ -196,7 +194,6 @@ struct rtrs_msg_conn_req {
>         __le16          version;
>         __le16          cid;
>         __le16          cid_num;
> -       __le16          recon_cnt;
>         uuid_t          sess_uuid;
>         uuid_t          paths_uuid;
>         u8              first_conn : 1;
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 4c883c57c2ef..e2ea09a8def7 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1712,7 +1712,6 @@ static int create_con(struct rtrs_srv_path *srv_path,
>  static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
>                                            struct rdma_cm_id *cm_id,
>                                            unsigned int con_num,
> -                                          unsigned int recon_cnt,
>                                            const uuid_t *uuid)
>  {
>         struct rtrs_srv_path *srv_path;
> @@ -1768,7 +1767,6 @@ static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
>
>         srv_path->s.con_num = con_num;
>         srv_path->s.irq_con_num = con_num;
> -       srv_path->s.recon_cnt = recon_cnt;
>         uuid_copy(&srv_path->s.uuid, uuid);
>         spin_lock_init(&srv_path->state_lock);
>         INIT_WORK(&srv_path->close_work, rtrs_srv_close_work);
> @@ -1818,7 +1816,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>         struct rtrs_srv_sess *srv;
>
>         u16 version, con_num, cid;
> -       u16 recon_cnt;
>         int err = -ECONNRESET;
>         bool alloc_path = false;
>
> @@ -1848,7 +1845,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>                 pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
>                 goto reject_w_err;
>         }
> -       recon_cnt = le16_to_cpu(msg->recon_cnt);
>         srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
>         if (IS_ERR(srv)) {
>                 err = PTR_ERR(srv);
> @@ -1885,8 +1881,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
>                         goto reject_w_err;
>                 }
>         } else {
> -               srv_path = __alloc_path(srv, cm_id, con_num, recon_cnt,
> -                                   &msg->sess_uuid);
> +               srv_path = __alloc_path(srv, cm_id, con_num, &msg->sess_uuid);
>                 if (IS_ERR(srv_path)) {
>                         mutex_unlock(&srv->paths_mutex);
>                         put_srv(srv);
> --
> 2.31.1
>
Jinpu Wang Nov. 15, 2022, 9:53 a.m. UTC | #2
On Tue, Nov 15, 2022 at 10:39 AM Haris Iqbal <haris.iqbal@ionos.com> wrote:
>
> On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> >
> > Seems the only relevant comment about recon_cnt is,
> >
> > /*
> >  * On every new session connections increase reconnect counter
> >  * to avoid clashes with previous sessions not yet closed
> >  * sessions on a server side.
> >  */
> >
> > However, it is not clear how the recon_cnt avoid clashed at these places
> > in the commit since no where checks it.
>
> It does seem redundant. This predates my time, so I don't know if
> there was a change which removed the usage of this. I tried to search
> in commit history, but couldn't.
>
> @Jinpu Your thoughts?
>
> >
> > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > ---
> >  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
> >  drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
> >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
> >  3 files changed, 1 insertion(+), 17 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > index 5ffc170dae8c..dcc8c041a141 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > @@ -1802,7 +1802,6 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
> >                 .version = cpu_to_le16(RTRS_PROTO_VER),
> >                 .cid = cpu_to_le16(con->c.cid),
> >                 .cid_num = cpu_to_le16(clt_path->s.con_num),
> > -               .recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
> >         };
> >         msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
> >         uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
> > @@ -2336,13 +2335,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
> >         unsigned int cid;
> >         int err;
> >
> > -       /*
> > -        * On every new session connections increase reconnect counter
> > -        * to avoid clashes with previous sessions not yet closed
> > -        * sessions on a server side.
> > -        */
> > -       clt_path->s.recon_cnt++;
> > -
> >         /* Establish all RDMA connections  */
> >         for (cid = 0; cid < clt_path->s.con_num; cid++) {
> >                 err = create_con(clt_path, cid);
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > index a2420eecaf5a..c4ddaeba1c59 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > @@ -109,7 +109,6 @@ struct rtrs_path {
> >         struct rtrs_con **con;
> >         unsigned int            con_num;
> >         unsigned int            irq_con_num;
> > -       unsigned int            recon_cnt;
> >         unsigned int            signal_interval;
> >         struct rtrs_ib_dev      *dev;
> >         int                     dev_ref;
> > @@ -177,7 +176,6 @@ struct rtrs_sg_desc {
> >   * @version:      RTRS protocol version
> >   * @cid:          Current connection id
> >   * @cid_num:      Number of connections per session
> > - * @recon_cnt:    Reconnections counter
> >   * @sess_uuid:    UUID of a session (path)
> >   * @paths_uuid:           UUID of a group of sessions (paths)
> >   *
> > @@ -196,7 +194,6 @@ struct rtrs_msg_conn_req {
> >         __le16          version;
> >         __le16          cid;
> >         __le16          cid_num;
> > -       __le16          recon_cnt;
> >         uuid_t          sess_uuid;
> >         uuid_t          paths_uuid;
> >         u8              first_conn : 1;
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > index 4c883c57c2ef..e2ea09a8def7 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > @@ -1712,7 +1712,6 @@ static int create_con(struct rtrs_srv_path *srv_path,
> >  static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
> >                                            struct rdma_cm_id *cm_id,
> >                                            unsigned int con_num,
> > -                                          unsigned int recon_cnt,
> >                                            const uuid_t *uuid)
> >  {
> >         struct rtrs_srv_path *srv_path;
> > @@ -1768,7 +1767,6 @@ static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
> >
> >         srv_path->s.con_num = con_num;
> >         srv_path->s.irq_con_num = con_num;
> > -       srv_path->s.recon_cnt = recon_cnt;
> >         uuid_copy(&srv_path->s.uuid, uuid);
> >         spin_lock_init(&srv_path->state_lock);
> >         INIT_WORK(&srv_path->close_work, rtrs_srv_close_work);
> > @@ -1818,7 +1816,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >         struct rtrs_srv_sess *srv;
> >
> >         u16 version, con_num, cid;
> > -       u16 recon_cnt;
> >         int err = -ECONNRESET;
> >         bool alloc_path = false;
> >
> > @@ -1848,7 +1845,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >                 pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
> >                 goto reject_w_err;
> >         }
> > -       recon_cnt = le16_to_cpu(msg->recon_cnt);
> >         srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> >         if (IS_ERR(srv)) {
> >                 err = PTR_ERR(srv);
> > @@ -1885,8 +1881,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> >                         goto reject_w_err;
> >                 }
> >         } else {
> > -               srv_path = __alloc_path(srv, cm_id, con_num, recon_cnt,
> > -                                   &msg->sess_uuid);
> > +               srv_path = __alloc_path(srv, cm_id, con_num, &msg->sess_uuid);
> >                 if (IS_ERR(srv_path)) {
> >                         mutex_unlock(&srv->paths_mutex);
> >                         put_srv(srv);
> > --
> > 2.31.1
> >
Jinpu Wang Nov. 15, 2022, 9:58 a.m. UTC | #3
On Tue, Nov 15, 2022 at 10:53 AM Jinpu Wang <jinpu.wang@ionos.com> wrote:
>
> On Tue, Nov 15, 2022 at 10:39 AM Haris Iqbal <haris.iqbal@ionos.com> wrote:
> >
> > On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
> > >
> > > Seems the only relevant comment about recon_cnt is,
> > >
> > > /*
> > >  * On every new session connections increase reconnect counter
> > >  * to avoid clashes with previous sessions not yet closed
> > >  * sessions on a server side.
> > >  */
> > >
> > > However, it is not clear how the recon_cnt avoid clashed at these places
> > > in the commit since no where checks it.
> >
> > It does seem redundant. This predates my time, so I don't know if
> > there was a change which removed the usage of this. I tried to search
> > in commit history, but couldn't.
> >
> > @Jinpu Your thoughts?
Sorry for sending an empty reply.
I checked a bit, and also can find a reason why the recon_cnt was
added, it is not checked on the server side.
I think it's ok to remove it. But we can simply remove the field in
rtrs_msg_conn_req, commented below.

> >
> > >
> > > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> > > ---
> > >  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
> > >  drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
> > >  drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
> > >  3 files changed, 1 insertion(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > > index 5ffc170dae8c..dcc8c041a141 100644
> > > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > > @@ -1802,7 +1802,6 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
> > >                 .version = cpu_to_le16(RTRS_PROTO_VER),
> > >                 .cid = cpu_to_le16(con->c.cid),
> > >                 .cid_num = cpu_to_le16(clt_path->s.con_num),
> > > -               .recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
> > >         };
> > >         msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
> > >         uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
> > > @@ -2336,13 +2335,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
> > >         unsigned int cid;
> > >         int err;
> > >
> > > -       /*
> > > -        * On every new session connections increase reconnect counter
> > > -        * to avoid clashes with previous sessions not yet closed
> > > -        * sessions on a server side.
> > > -        */
> > > -       clt_path->s.recon_cnt++;
> > > -
> > >         /* Establish all RDMA connections  */
> > >         for (cid = 0; cid < clt_path->s.con_num; cid++) {
> > >                 err = create_con(clt_path, cid);
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > > index a2420eecaf5a..c4ddaeba1c59 100644
> > > --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > > @@ -109,7 +109,6 @@ struct rtrs_path {
> > >         struct rtrs_con **con;
> > >         unsigned int            con_num;
> > >         unsigned int            irq_con_num;
> > > -       unsigned int            recon_cnt;
> > >         unsigned int            signal_interval;
> > >         struct rtrs_ib_dev      *dev;
> > >         int                     dev_ref;
> > > @@ -177,7 +176,6 @@ struct rtrs_sg_desc {
> > >   * @version:      RTRS protocol version
> > >   * @cid:          Current connection id
> > >   * @cid_num:      Number of connections per session
> > > - * @recon_cnt:    Reconnections counter
> > >   * @sess_uuid:    UUID of a session (path)
> > >   * @paths_uuid:           UUID of a group of sessions (paths)
> > >   *
> > > @@ -196,7 +194,6 @@ struct rtrs_msg_conn_req {
> > >         __le16          version;
> > >         __le16          cid;
> > >         __le16          cid_num;
> > > -       __le16          recon_cnt;
> > >         uuid_t          sess_uuid;
> > >         uuid_t          paths_uuid;
> > >         u8              first_conn : 1;
We can remove it from protocol, this break compatibility with older kernel.
> > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > index 4c883c57c2ef..e2ea09a8def7 100644
> > > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > > @@ -1712,7 +1712,6 @@ static int create_con(struct rtrs_srv_path *srv_path,
> > >  static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
> > >                                            struct rdma_cm_id *cm_id,
> > >                                            unsigned int con_num,
> > > -                                          unsigned int recon_cnt,
> > >                                            const uuid_t *uuid)
> > >  {
> > >         struct rtrs_srv_path *srv_path;
> > > @@ -1768,7 +1767,6 @@ static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
> > >
> > >         srv_path->s.con_num = con_num;
> > >         srv_path->s.irq_con_num = con_num;
> > > -       srv_path->s.recon_cnt = recon_cnt;
> > >         uuid_copy(&srv_path->s.uuid, uuid);
> > >         spin_lock_init(&srv_path->state_lock);
> > >         INIT_WORK(&srv_path->close_work, rtrs_srv_close_work);
> > > @@ -1818,7 +1816,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > >         struct rtrs_srv_sess *srv;
> > >
> > >         u16 version, con_num, cid;
> > > -       u16 recon_cnt;
> > >         int err = -ECONNRESET;
> > >         bool alloc_path = false;
> > >
> > > @@ -1848,7 +1845,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > >                 pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
> > >                 goto reject_w_err;
> > >         }
> > > -       recon_cnt = le16_to_cpu(msg->recon_cnt);
> > >         srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> > >         if (IS_ERR(srv)) {
> > >                 err = PTR_ERR(srv);
> > > @@ -1885,8 +1881,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> > >                         goto reject_w_err;
> > >                 }
> > >         } else {
> > > -               srv_path = __alloc_path(srv, cm_id, con_num, recon_cnt,
> > > -                                   &msg->sess_uuid);
> > > +               srv_path = __alloc_path(srv, cm_id, con_num, &msg->sess_uuid);
> > >                 if (IS_ERR(srv_path)) {
> > >                         mutex_unlock(&srv->paths_mutex);
> > >                         put_srv(srv);
> > > --
> > > 2.31.1
> > >
Guoqing Jiang Nov. 15, 2022, 10:33 a.m. UTC | #4
On 11/15/22 5:58 PM, Jinpu Wang wrote:
> On Tue, Nov 15, 2022 at 10:53 AM Jinpu Wang <jinpu.wang@ionos.com> wrote:
>> On Tue, Nov 15, 2022 at 10:39 AM Haris Iqbal <haris.iqbal@ionos.com> wrote:
>>> On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>>>> Seems the only relevant comment about recon_cnt is,
>>>>
>>>> /*
>>>>   * On every new session connections increase reconnect counter
>>>>   * to avoid clashes with previous sessions not yet closed
>>>>   * sessions on a server side.
>>>>   */
>>>>
>>>> However, it is not clear how the recon_cnt avoid clashed at these places
>>>> in the commit since no where checks it.
>>> It does seem redundant. This predates my time, so I don't know if
>>> there was a change which removed the usage of this. I tried to search
>>> in commit history, but couldn't.
>>>
>>> @Jinpu Your thoughts?
> Sorry for sending an empty reply.
> I checked a bit, and also can find a reason why the recon_cnt was
> added, it is not checked on the server side.
> I think it's ok to remove it. But we can simply remove the field in
> rtrs_msg_conn_req, commented below.
>
>>>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
>>>> ---
>>>>   drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
>>>>   drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
>>>>   drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
>>>>   3 files changed, 1 insertion(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
>>>> index 5ffc170dae8c..dcc8c041a141 100644
>>>> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
>>>> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
>>>> @@ -1802,7 +1802,6 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
>>>>                  .version = cpu_to_le16(RTRS_PROTO_VER),
>>>>                  .cid = cpu_to_le16(con->c.cid),
>>>>                  .cid_num = cpu_to_le16(clt_path->s.con_num),
>>>> -               .recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
>>>>          };
>>>>          msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
>>>>          uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
>>>> @@ -2336,13 +2335,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
>>>>          unsigned int cid;
>>>>          int err;
>>>>
>>>> -       /*
>>>> -        * On every new session connections increase reconnect counter
>>>> -        * to avoid clashes with previous sessions not yet closed
>>>> -        * sessions on a server side.
>>>> -        */
>>>> -       clt_path->s.recon_cnt++;
>>>> -
>>>>          /* Establish all RDMA connections  */
>>>>          for (cid = 0; cid < clt_path->s.con_num; cid++) {
>>>>                  err = create_con(clt_path, cid);
>>>> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
>>>> index a2420eecaf5a..c4ddaeba1c59 100644
>>>> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
>>>> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
>>>> @@ -109,7 +109,6 @@ struct rtrs_path {
>>>>          struct rtrs_con **con;
>>>>          unsigned int            con_num;
>>>>          unsigned int            irq_con_num;
>>>> -       unsigned int            recon_cnt;
>>>>          unsigned int            signal_interval;
>>>>          struct rtrs_ib_dev      *dev;
>>>>          int                     dev_ref;
>>>> @@ -177,7 +176,6 @@ struct rtrs_sg_desc {
>>>>    * @version:      RTRS protocol version
>>>>    * @cid:          Current connection id
>>>>    * @cid_num:      Number of connections per session
>>>> - * @recon_cnt:    Reconnections counter
>>>>    * @sess_uuid:    UUID of a session (path)
>>>>    * @paths_uuid:           UUID of a group of sessions (paths)
>>>>    *
>>>> @@ -196,7 +194,6 @@ struct rtrs_msg_conn_req {
>>>>          __le16          version;
>>>>          __le16          cid;
>>>>          __le16          cid_num;
>>>> -       __le16          recon_cnt;
>>>>          uuid_t          sess_uuid;
>>>>          uuid_t          paths_uuid;
>>>>          u8              first_conn : 1;
> We can remove it from protocol, this break compatibility with older kernel.

Can we bump up RTRS_PROTO_VER_MINOR for the compatibility? Otherwise
those structs are immutable forever.

Thanks,
Guoqing
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 5ffc170dae8c..dcc8c041a141 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1802,7 +1802,6 @@  static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
 		.version = cpu_to_le16(RTRS_PROTO_VER),
 		.cid = cpu_to_le16(con->c.cid),
 		.cid_num = cpu_to_le16(clt_path->s.con_num),
-		.recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
 	};
 	msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
 	uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
@@ -2336,13 +2335,6 @@  static int init_conns(struct rtrs_clt_path *clt_path)
 	unsigned int cid;
 	int err;
 
-	/*
-	 * On every new session connections increase reconnect counter
-	 * to avoid clashes with previous sessions not yet closed
-	 * sessions on a server side.
-	 */
-	clt_path->s.recon_cnt++;
-
 	/* Establish all RDMA connections  */
 	for (cid = 0; cid < clt_path->s.con_num; cid++) {
 		err = create_con(clt_path, cid);
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index a2420eecaf5a..c4ddaeba1c59 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -109,7 +109,6 @@  struct rtrs_path {
 	struct rtrs_con	**con;
 	unsigned int		con_num;
 	unsigned int		irq_con_num;
-	unsigned int		recon_cnt;
 	unsigned int		signal_interval;
 	struct rtrs_ib_dev	*dev;
 	int			dev_ref;
@@ -177,7 +176,6 @@  struct rtrs_sg_desc {
  * @version:	   RTRS protocol version
  * @cid:	   Current connection id
  * @cid_num:	   Number of connections per session
- * @recon_cnt:	   Reconnections counter
  * @sess_uuid:	   UUID of a session (path)
  * @paths_uuid:	   UUID of a group of sessions (paths)
  *
@@ -196,7 +194,6 @@  struct rtrs_msg_conn_req {
 	__le16		version;
 	__le16		cid;
 	__le16		cid_num;
-	__le16		recon_cnt;
 	uuid_t		sess_uuid;
 	uuid_t		paths_uuid;
 	u8		first_conn : 1;
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 4c883c57c2ef..e2ea09a8def7 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1712,7 +1712,6 @@  static int create_con(struct rtrs_srv_path *srv_path,
 static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
 					   struct rdma_cm_id *cm_id,
 					   unsigned int con_num,
-					   unsigned int recon_cnt,
 					   const uuid_t *uuid)
 {
 	struct rtrs_srv_path *srv_path;
@@ -1768,7 +1767,6 @@  static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
 
 	srv_path->s.con_num = con_num;
 	srv_path->s.irq_con_num = con_num;
-	srv_path->s.recon_cnt = recon_cnt;
 	uuid_copy(&srv_path->s.uuid, uuid);
 	spin_lock_init(&srv_path->state_lock);
 	INIT_WORK(&srv_path->close_work, rtrs_srv_close_work);
@@ -1818,7 +1816,6 @@  static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 	struct rtrs_srv_sess *srv;
 
 	u16 version, con_num, cid;
-	u16 recon_cnt;
 	int err = -ECONNRESET;
 	bool alloc_path = false;
 
@@ -1848,7 +1845,6 @@  static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 		pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
 		goto reject_w_err;
 	}
-	recon_cnt = le16_to_cpu(msg->recon_cnt);
 	srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
 	if (IS_ERR(srv)) {
 		err = PTR_ERR(srv);
@@ -1885,8 +1881,7 @@  static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
 			goto reject_w_err;
 		}
 	} else {
-		srv_path = __alloc_path(srv, cm_id, con_num, recon_cnt,
-				    &msg->sess_uuid);
+		srv_path = __alloc_path(srv, cm_id, con_num, &msg->sess_uuid);
 		if (IS_ERR(srv_path)) {
 			mutex_unlock(&srv->paths_mutex);
 			put_srv(srv);