Re: [PATCH rdma-next] IB/cma: Define option to set ack timeout and pack tos_set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 20, 2019 at 01:08:53AM +0000, Parav Pandit wrote:
>
>
> > -----Original Message-----
> > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> > owner@xxxxxxxxxxxxxxx> On Behalf Of Leon Romanovsky
> > Sent: Thursday, January 17, 2019 11:54 AM
> > To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe
> > <jgg@xxxxxxxxxxxx>
> > Cc: Danit Goldberg <danitg@xxxxxxxxxxxx>; RDMA mailing list <linux-
> > rdma@xxxxxxxxxxxxxxx>; Moni Shoua <monis@xxxxxxxxxxxx>; Leon
> > Romanovsky <leonro@xxxxxxxxxxxx>
> > Subject: [PATCH rdma-next] IB/cma: Define option to set ack timeout and
> > pack tos_set
> >
> > From: Danit Goldberg <danitg@xxxxxxxxxxxx>
> >
> > Define new option in 'rdma_set_option' to override calculated QP timeout
> > when requested to provide QP attributes to modify a QP.
> >
> > At the same time, pack tos_set to be bitfield.
> >
> > Signed-off-by: Danit Goldberg <danitg@xxxxxxxxxxxx>
> > Reviewed-by: Moni Shoua <monis@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > ---
> >  drivers/infiniband/core/cma.c      | 14 ++++++++++++++
> >  drivers/infiniband/core/cma_priv.h |  4 +++-
> >  drivers/infiniband/core/ucma.c     |  7 +++++++
> >  include/rdma/rdma_cm.h             |  7 +++++++
> >  include/uapi/rdma/rdma_user_cm.h   |  4 ++++
> >  5 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index 3880fbb3bee7..d4c861f64194 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -888,6 +888,7 @@ struct rdma_cm_id *__rdma_create_id(struct net
> > *net,
> >  	id_priv->id.ps = ps;
> >  	id_priv->id.qp_type = qp_type;
> >  	id_priv->tos_set = false;
> > +	id_priv->timeout_set = false;
> >  	id_priv->gid_type = IB_GID_TYPE_IB;
> >  	spin_lock_init(&id_priv->lock);
> >  	mutex_init(&id_priv->qp_mutex);
> > @@ -1130,6 +1131,9 @@ int rdma_init_qp_attr(struct rdma_cm_id *id,
> > struct ib_qp_attr *qp_attr,
> >  	} else
> >  		ret = -ENOSYS;
> >
> > +	if ((*qp_attr_mask & IB_QP_TIMEOUT) && id_priv->timeout_set)
> > +		qp_attr->timeout = id_priv->timeout;
> > +
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(rdma_init_qp_attr);
> > @@ -2490,6 +2494,16 @@ void rdma_set_service_type(struct rdma_cm_id
> > *id, int tos)  }  EXPORT_SYMBOL(rdma_set_service_type);
> >
> > +void rdma_set_ack_timeout(struct rdma_cm_id *id, u8 timeout) {
> > +	struct rdma_id_private *id_priv;
> > +
> > +	id_priv = container_of(id, struct rdma_id_private, id);
> > +	id_priv->timeout = timeout;
> > +	id_priv->timeout_set = true;
> > +}
> > +EXPORT_SYMBOL(rdma_set_ack_timeout);
> > +
> Please change to return error if the cm_id when qp_type != RC.
> Or move is_qp_type_connected() from core/verbs.c to core_priv.h and reuse here.
>
> Also validating by modifying is not very elegant way.
> I actually have a patch that extend several qp params via netlink which will allows to verify this as well.
> I posted in internal gerrit. Please bundle it with v1 version of it.
> I didn't have iproute2 enhanced so was holding to queue to Leon, but its good to do now in along with this patch.

kdoc and checks of rdma_cm_id are very good suggestions and we will do them,
but netlink is a little bit out-of-scope for this patch. As you said, it
will be separate patch together with iproute2.

Thanks

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux