On Wed, Mar 21, 2018 at 05:16:35PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav@xxxxxxxxxxxx>
>
> Currently CM request for RoCE follows following flow.
> rdma_create_id()
> rdma_resolve_addr()
> rdma_resolve_route()
> For RC QPs:
> rdma_connect()
> ->cma_connect_ib()
> ->ib_send_cm_req()
> ->cm_init_av_by_path()
> ->ib_init_ah_attr_from_path()
> For UD QPs:
> rdma_connect()
> ->cma_resolve_ib_udp()
> ->ib_send_cm_sidr_req()
> ->cm_init_av_by_path()
> ->ib_init_ah_attr_from_path()
>
> In both the flows, route is already resolved before sending CM requests.
> Therefore, code is refactored to avoid resolving route second time in
> ib_cm layer.
> ib_init_ah_attr_from_path() is extended to resolve route when it is not
> yet resolved for RoCE link layer. This is achieved by caller setting
> route_resolved field in path record whenever it has route already
> resolved.
>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/cm.c | 7 ++++++-
> drivers/infiniband/core/cma.c | 1 +
> drivers/infiniband/core/sa_query.c | 11 ++++++++---
> include/rdma/ib_sa.h | 8 ++++++++
> 4 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 4cc0fe6a29ff..71a26dc798b0 100644
> +++ b/drivers/infiniband/core/cm.c
> @@ -1943,9 +1943,11 @@ static int cm_req_handler(struct cm_work *work)
> work->path[1].rec_type = work->path[0].rec_type;
> cm_format_paths_from_req(req_msg, &work->path[0],
> &work->path[1]);
> - if (cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE)
> + if (cm_id_priv->av.ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE) {
> sa_path_set_dmac(&work->path[0],
> cm_id_priv->av.ah_attr.roce.dmac);
> + work->path[0].roce.route_resolved = false;
> + }
Why this hunk? path[0] was memset to 0 up above, don't see any obvious
path that sets route_resolved = true between here and the memset?
> work->path[0].hop_limit = grh->hop_limit;
> ret = cm_init_av_by_path(&work->path[0], &cm_id_priv->av,
> cm_id_priv);
> @@ -1967,6 +1969,9 @@ static int cm_req_handler(struct cm_work *work)
> goto rejected;
> }
> if (cm_req_has_alt_path(req_msg)) {
> + if (sa_path_is_roce(&work->path[1]))
> + work->path[1].roce.route_resolved = false;
> +
> ret = cm_init_av_by_path(&work->path[1], &cm_id_priv->alt_av,
> cm_id_priv);
ditto
> @@ -1327,9 +1329,12 @@ int ib_init_ah_attr_from_path(struct ib_device *device, u8 port_num,
> rdma_ah_set_static_rate(ah_attr, rec->rate);
>
> if (sa_path_is_roce(rec)) {
> - ret = roce_resolve_route_from_path(device, port_num, rec);
> - if (ret)
> - return ret;
> + if (!rec->roce.route_resolved) {
> + ret = roce_resolve_route_from_path(device, port_num,
> + rec);
Any particular reason to just not exit from
roce_resolve_route_from_path() if route_resolved?
Otherwise seems reasonable to me
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
