From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 6B3973238; Fri, 15 Sep 2017 10:39:08 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Sep 2017 01:39:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,396,1500966000"; d="scan'208";a="900524969" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by FMSMGA003.fm.intel.com with ESMTP; 15 Sep 2017 01:39:05 -0700 To: =?UTF-8?Q?N=c3=a9lio_Laranjeiro?= , Shahaf Shuler , Yongseok Koh Cc: Adrien Mazarguil , "dev@dpdk.org" , "stable@dpdk.org" References: <20170831162706.30899-1-yskoh@mellanox.com> <20170904140107.nkr565m266sw5cyf@localhost> <20170911221743.GB1922@yongseok-MBP.local> <20170912072411.GO14138@autoinstall.dev.6wind.com> <30C13A14-CB69-45B0-9DC8-EBE8048098B7@mellanox.com> <20170913072606.GA29920@autoinstall.dev.6wind.com> From: Ferruh Yigit Message-ID: Date: Fri, 15 Sep 2017 09:39:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170913072606.GA29920@autoinstall.dev.6wind.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix calculating TSO inline size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Sep 2017 08:39:08 -0000 On 9/13/2017 8:26 AM, Nélio Laranjeiro wrote: > On Wed, Sep 13, 2017 at 05:05:14AM +0000, Shahaf Shuler wrote: >> Tuesday, September 12, 2017 9:34 PM, Nélio Laranjeiro: >>>> On Sep 12, 2017, at 12:24 AM, Nélio Laranjeiro >>>>>> Is not it dangerous to assume inl will always be 4 bytes long? Why >>>>>> not writing the real value instead? >>>>> That was for readability of the code and uint32_t will be always >>>>> 4bytes. But for better readability, it should be 'inline_header' >>>>> instead of 'inl' though. I'm also okay with using "4" as well. Which way do >>> you prefer? >>>> >>>> I agree on both, I was not clear enough to explain my thought, if for >>>> some reason the inl moves from uint32_t to uint16_t without touching >>>> the sizeof later, it will cause an issue. >>> >>> I tried to change the sizeof but I found that there are more "sizeof(inl)" in the >>> following lines. Changing all the sizeof would be beyond the scope of this >>> patch. So, how about leaving it as is for consistency? >> >> The inline segment format is not expected to change so easily. It is parsed >> by the HW and HW maintains backward compatibility for all of the WQE >> structures. > > We are not talking here about the hardware behavior but about wrong ways to > define hardware values which should be used trough define and not by size of > variable which can be wrongly modified. > If the hardware inline_header size of 4, it should be defined as is. > > I won't block this patch as it is not the single place where this sizeof is > used, but it should be replaced as soon as possible by a define to avoid wrong > behaviors in the future. > > Acked-by: Nelio Laranjeiro Applied to dpdk-next-net/master, thanks.