From patchwork Wed Mar 21 22:44:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 10300505 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5C8BC60385 for ; Wed, 21 Mar 2018 22:44:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 481522903D for ; Wed, 21 Mar 2018 22:44:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3C3DF2990D; Wed, 21 Mar 2018 22:44:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 69FD029919 for ; Wed, 21 Mar 2018 22:44:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754018AbeCUWoq (ORCPT ); Wed, 21 Mar 2018 18:44:46 -0400 Received: from mail-pl0-f68.google.com ([209.85.160.68]:34287 "EHLO mail-pl0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753691AbeCUWon (ORCPT ); Wed, 21 Mar 2018 18:44:43 -0400 Received: by mail-pl0-f68.google.com with SMTP id u11-v6so4063626plq.1 for ; Wed, 21 Mar 2018 15:44:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=KbtvSTDEgeYlxQCuaaxsLHdGF9+oN0ftc93GfH1fdhY=; b=K3EDRzkkGsWafF7f8J8+QKSHUThRZ5gP3O5NOvl/Bwz3W1J0ws7LWakkXkIfnaETh7 tThGar5frfCIEFrm377OfMD2kDc+UMqL8CWDpMTFmof58d+UlJVRVoNYvynbLa834J6e TpngNXFDqMwGEiGiNcFTjN0AwdVzzj6Kvv0PCyMn8VhNV/+yBgRNZdgdJJbVJ4EiMVpA CDh51w0izxCNiplDabWIXMGpMkEWr3JGXDT/6EjBTxpMbhZYtT68SBowNgAht+EX7Phq +3xDXiUGlrdnvwloqSlxGj6wrwfJCQOJp9WYStCo1iir1k6lUc1nLK9R3tBB3EF9ymdi 9I2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=KbtvSTDEgeYlxQCuaaxsLHdGF9+oN0ftc93GfH1fdhY=; b=gR1is9utONzI7vqNL7NRGOG2EUqHPb5UsSquZC12Ufb562MKBKun9Yhxo6EUZJgaBK 1ocMlCCdXBVM71ywXwy9+iz6NbJxUt5cGYLbHpJEUkND33aP4rybQ8WNUJGsgf5H3BKp MDzLGnD6TjSsOc7SA/biw/Q0aC3NQkZb2IMyu3t7Qidlsl2IjsrfxEOdTp7JsAZk0hLb CauengxRIbPT2JWVFV76css5cE6n0cFkMc0O9Z75V/t2SpE9JGkjwLccyWk7ATDnTx8a dUHYyWP4cTD1hr4mY7Ani88o2uySm6lXs4zNwdBwzEcpptotKS4fZkx4l51rmU52Qd19 d93Q== X-Gm-Message-State: AElRT7Hh7rOkC1hAKzQxpXupYKBY/8cX/zkM14tkFbixqYwRV4Zec2dD +vGalKtpVJCXMGA1QZY230WKNFoEPvc= X-Google-Smtp-Source: AG47ELuD2TycFPfWbNeIrm1vwrGJ60rr+eemeID23LbVT5trnMy00+A+J3RNHHHYWsh3VwObiEgcVQ== X-Received: by 2002:a17:902:d81:: with SMTP id 1-v6mr21935034plv.324.1521672282555; Wed, 21 Mar 2018 15:44:42 -0700 (PDT) Received: from ziepe.ca (S010614cc2056d97f.ed.shawcable.net. [174.3.196.123]) by smtp.gmail.com with ESMTPSA id r20sm11697265pff.165.2018.03.21.15.44.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 21 Mar 2018 15:44:41 -0700 (PDT) Received: from jgg by mlx.ziepe.ca with local (Exim 4.86_2) (envelope-from ) id 1eymTI-0008Tk-P1; Wed, 21 Mar 2018 16:44:40 -0600 Date: Wed, 21 Mar 2018 16:44:40 -0600 From: Jason Gunthorpe To: Parav Pandit Cc: Leon Romanovsky , Doug Ledford , Leon Romanovsky , RDMA mailing list , Mark Bloch Subject: Re: [PATCH rdma-next 1/2] IB/cma: Resolve route only while receiving CM requests Message-ID: <20180321224440.GL22899@ziepe.ca> References: <20180321151636.12381-1-leon@kernel.org> <20180321151636.12381-2-leon@kernel.org> <20180321215059.GA26123@ziepe.ca> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Mar 21, 2018 at 10:27:02PM +0000, Parav Pandit wrote: > > On Wed, Mar 21, 2018 at 05:16:35PM +0200, Leon Romanovsky wrote: > > > From: Parav Pandit > > > > > > 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 > > > Signed-off-by: Leon Romanovsky > > > 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? > > > Right. It is not needed due to memset. > Since CM path appears in general little complicated, setting it > explicitly gives more clarity, but otherwise it can be omitted in > both of these places. I'm not sure it is clear.. I read the code and wonder why it is there, and cannot figure it out. I expect route_resolved = false to be placed at any spot that invalidates the route lookup cached in the struct. So lets put it in cm_format_paths_from_req which clearly does invalidate any cached route? It is still unnecessary, but at least it is in a place that makes some sense.. > > > @@ -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? > > > Do you mean, > Can if (!rec->roce.route_resolved) reside inside roce_resolve_route_from_path()? > > If I understood you right, yes, it can be. But it is only checked > once, so placed it outside. But it is ok to have it inside too. Okay, lets do that, if we have a function called from only one place it may as well do everything. Then it has a nice internal symmetry. So like this? --- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index 4cc0fe6a29ff08..38d79bc1bf78f0 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -1543,6 +1543,8 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg, cm_req_get_primary_local_ack_timeout(req_msg); primary_path->packet_life_time -= (primary_path->packet_life_time > 0); primary_path->service_id = req_msg->service_id; + if (sa_path_is_roce(primary_path)) + primary_path->roce.route_resolved = false; if (cm_req_has_alt_path(req_msg)) { alt_path->dgid = req_msg->alt_local_gid; @@ -1562,6 +1564,9 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg, cm_req_get_alt_local_ack_timeout(req_msg); alt_path->packet_life_time -= (alt_path->packet_life_time > 0); alt_path->service_id = req_msg->service_id; + + if (sa_path_is_roce(alt_path)) + alt_path->roce.route_resolved = false; } cm_format_path_lid_from_req(req_msg, primary_path, alt_path); } diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 34fa0507ed4ff1..8512f633efd642 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -2506,6 +2506,7 @@ cma_iboe_set_path_rec_l2_fields(struct rdma_id_private *id_priv) gid_type = ib_network_to_gid_type(addr->dev_addr.network); route->path_rec->rec_type = sa_conv_gid_to_pathrec_type(gid_type); + route->path_rec->roce.route_resolved = true; sa_path_set_ndev(route->path_rec, addr->dev_addr.net); sa_path_set_ifindex(route->path_rec, ndev->ifindex); sa_path_set_dmac(route->path_rec, addr->dev_addr.dst_dev_addr); diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c index 1cfec68c79115a..a61ec7e33613e9 100644 --- a/drivers/infiniband/core/sa_query.c +++ b/drivers/infiniband/core/sa_query.c @@ -1248,6 +1248,9 @@ roce_resolve_route_from_path(struct ib_device *device, u8 port_num, } sgid_addr, dgid_addr; int ret; + if (rec->roce.route_resolved) + return 0; + if (!device->get_netdev) return -EOPNOTSUPP; @@ -1287,6 +1290,8 @@ roce_resolve_route_from_path(struct ib_device *device, u8 port_num, dev_put(ndev); done: dev_put(idev); + if (!ret) + rec->roce.route_resolved = true; return ret; } diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h index 82b8e59af14ab7..bacb144f778024 100644 --- a/include/rdma/ib_sa.h +++ b/include/rdma/ib_sa.h @@ -163,7 +163,15 @@ struct sa_path_rec_ib { u8 raw_traffic; }; +/** + * struct sa_path_rec_roce - RoCE specific portion of the path record entry + * @route_resolved: When set, it indicates that this route is already + * resolved for this path record entry. + * @dmac: Destination mac address for the given DGID entry + * of the path record entry. + */ struct sa_path_rec_roce { + bool route_resolved; u8 dmac[ETH_ALEN]; /* ignored in IB */ int ifindex;