From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 09D1EC433ED for ; Tue, 20 Apr 2021 05:15:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BE63F613B0 for ; Tue, 20 Apr 2021 05:15:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229724AbhDTFQO (ORCPT ); Tue, 20 Apr 2021 01:16:14 -0400 Received: from mail.kernel.org ([198.145.29.99]:34160 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229588AbhDTFQO (ORCPT ); Tue, 20 Apr 2021 01:16:14 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 99B5B613AE; Tue, 20 Apr 2021 05:15:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1618895743; bh=dhlrIivZxquO2S27t8UwtNK40E82mVYLo2S9s3w+VQs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=MjFDFFy7pA2EJpP9o3/PWq6rJ+fd3Lq8uk8C3NuqPgQKf2Jz+gME+UYSMSMqPVXnw 2t8U5clp3BiQnimEVW2aSV8uchZD4x8RhKHM/ZlWHQA0KuuThFt2nS0sQ8y4zUScqG A0FTHMRzLAbR+8wlHtqpoPN0SneIZto3383DoKMo5NDBSvK2LB8YTMY5by+L6g8aSt /1cRMK/XMARXXy9+DcwiQ1PRHnHYaihH5xS/o/4yJfOTh1DWfbMk+3IT4oo337rfRA MaELR+9OqluL0DR3Rc1W8zYl0wtuPTNM/qXfDwJbOnbdCI34gCne2M7vRbt8GnZJBQ UZOV4MQ6IXEdQ== Date: Tue, 20 Apr 2021 08:15:39 +0300 From: Leon Romanovsky To: "Wan, Kaike" Cc: Dennis Dalessandro , "dledford@redhat.com" , "jgg@ziepe.ca" , "linux-rdma@vger.kernel.org" Subject: Re: [PATCH for-next 06/10] rdma: Set physical MTU for query_port function Message-ID: References: <1617026056-50483-1-git-send-email-dennis.dalessandro@cornelisnetworks.com> <1617026056-50483-7-git-send-email-dennis.dalessandro@cornelisnetworks.com> <44ca5d0e-7aa4-5a9a-8f3b-d30454a58fb4@cornelisnetworks.com> <8d987675-09f3-542c-a921-072f19243e08@cornelisnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Mon, Apr 19, 2021 at 01:09:13PM +0000, Wan, Kaike wrote: > > > > -----Original Message----- > > From: Leon Romanovsky > > Sent: Monday, April 19, 2021 8:29 AM > > To: Wan, Kaike > > Cc: Dennis Dalessandro ; > > dledford@redhat.com; jgg@ziepe.ca; linux-rdma@vger.kernel.org > > Subject: Re: [PATCH for-next 06/10] rdma: Set physical MTU for query_port > > function > > > > On Mon, Apr 19, 2021 at 12:20:33PM +0000, Wan, Kaike wrote: > > > > > > > > > > -----Original Message----- > > > > From: Dennis Dalessandro > > > > Sent: Thursday, April 08, 2021 8:31 AM > > > > To: Leon Romanovsky > > > > Cc: dledford@redhat.com; jgg@ziepe.ca; linux-rdma@vger.kernel.org; > > > > Wan, Kaike > > > > Subject: Re: [PATCH for-next 06/10] rdma: Set physical MTU for > > > > query_port function > > > > > > > > On 4/8/2021 8:14 AM, Leon Romanovsky wrote: > > > > > On Thu, Apr 08, 2021 at 08:06:46AM -0400, Dennis Dalessandro wrote: > > > > >> On 4/1/2021 4:42 AM, Leon Romanovsky wrote: > > > > >>> On Mon, Mar 29, 2021 at 09:54:12AM -0400, > > > > dennis.dalessandro@cornelisnetworks.com wrote: > > > > >>>> From: Kaike Wan > > > > >>>> > > > > >>>> This is a follow on patch to add a phys_mtu field to the > > > > >>>> ib_port_attr structure to indicate the maximum physical MTU the > > > > >>>> underlying device supports. > > > > >>>> > > > > >>>> Extends the following: > > > > >>>> commit 6d72344cf6c4 ("IB/ipoib: Increase ipoib Datagram mode > > > > >>>> MTU's upper limit") > > > > >>>> > > > > >>>> Reviewed-by: Mike Marciniszyn > > > > >>>> > > > > >>>> Signed-off-by: Kaike Wan > > > > >>>> Signed-off-by: Dennis Dalessandro > > > > >>>> > > > > >>>> --- > > > > >>>> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/cxgb4/provider.c | 1 + > > > > >>>> drivers/infiniband/hw/efa/efa_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/hns/hns_roce_main.c | 1 + > > > > >>>> drivers/infiniband/hw/i40iw/i40iw_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/mlx4/main.c | 1 + > > > > >>>> drivers/infiniband/hw/mlx5/mad.c | 1 + > > > > >>>> drivers/infiniband/hw/mlx5/main.c | 2 ++ > > > > >>>> drivers/infiniband/hw/mthca/mthca_provider.c | 1 + > > > > >>>> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/qib/qib_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 1 + > > > > >>>> drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 1 + > > > > >>>> drivers/infiniband/sw/siw/siw_verbs.c | 1 + > > > > >>>> drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +- > > > > >>>> include/rdma/ib_verbs.h | 17 ----------------- > > > > >>>> 16 files changed, 16 insertions(+), 18 deletions(-) > > > > >>> > > > > >>> But why? What will it give us that almost all drivers have same > > > > >>> props->phys_mtu = ib_mtu_enum_to_int(props->max_mtu); line? > > > > >>> > > > > >> > > > > >> Almost is not all. Alternative idea to convey this? Seemed like a > > > > >> sensible thing to at least have support for but open to other > > approaches. > > > > > > > > > > What about leave it as is? > > > > > > > > > > I'm struggling to get the rationale behind this patch., the code > > > > > already works and set the phys_mtu correctly, isn't it? > > > > > > > > I see what you are saying now. Kaike, correct me if I'm wrong, but > > > > the intent of this patch is just to make everything behave the same > > > > in the sense that a device could have a different physical MTU. The > > > > field got added to the ib_port_attr previously so this is giving it an initial > > value vs leaving it unset. > > > [Wan, Kaike] Correct. > > > > No one is using this "phys_mtu" field, except one place in ipoib. > [Wan, Kaike] Since phys_mtu was introduced into ib_port_attr and every query_port call will return it in ib_port_attr. Rather than leaving it set to 0, setting it correctly in each hw driver seem more reasonable to me. Yes, if the drivers set this field differently, it is not the case here. All of them (except one) set same value. If you really want to have phys_mtu be correct all the time, change query_port to set it. Something like that: query_port(): if (!phys_mtu) phys_mtu = ib_mtu_enum_to_int(..) Thanks > > > > > Thanks > > > > > > > > > > > > > -Denny > > >