From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id 1DED5133F for ; Wed, 15 Mar 2017 21:15:54 +0100 (CET) Received: by mail-wm0-f48.google.com with SMTP id n11so32167279wma.0 for ; Wed, 15 Mar 2017 13:15:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=TNrOXSBz9QksEx6E9utkRQPjBqM+ROHcS0eOwx3hVTY=; b=RGBWzuJrik1nNAXwHwAI8EaJnpPorKFs9U/F4GqYNHXKniHGf8mozmtsN9L7UGfgp7 NzlNNJBhIwaduT07Ye8/GcSzij+2NaskAnllrPER0Tf7DsxtOwlqyNepPIGowZVZwc2z YHda8De+Xm5JrCY0G0WS6knHYXd3ZM+8cyZpJBVSOuFuuRgX4kItUJUwCw130FBXswJI eLHA72HYD/ikimok5Yfp3LFzivdTUd72tV9YVH58Q2p7AvNFESXmyZ4Wn4dlWDA8niAC JpNidfVauHFhEMUoMaMVdbOzbFtt81qXy6u6+ZwU9vWJMu5De9uUTKqqHeSPQ7YFfO+3 qxbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=TNrOXSBz9QksEx6E9utkRQPjBqM+ROHcS0eOwx3hVTY=; b=jtcWuw8xeMtpALMXzzWX546/otwgAR63V7pBXvmjzHRdE0VVfrz9hSPf7XZolWzll1 GCtGoOJfgHxNkcnpwl1Y+1edaLAix8NBL9IMiODmFALUaQuOpcDlGmyoNO/AhWK/kwjG HD5Dn9Z2acwc5G+MuAan/DHYMoqDozdPQd7FGGe5R5MGU9D9FRuBgvk4nHRgF3ecK3cc o7X6odOX1id7nL1oEXG8nkYw63Th2Ieo2JJbGI6k5vEfaceCinXXA2j4D4gyNeUdgwVD vFv/VLlHIKLvPKWWD4Y7oghjRZDRen6u7/11L023K2IWg1RjLjFJ9OHqGlUUEM9XQbiF MVFQ== X-Gm-Message-State: AFeK/H2TS3mSjJd0Z+veZ1VaJ719cGdNcOzWkEiClATew1SpQedeivBT1fjsqOdrSEeQ8ApL X-Received: by 10.28.198.132 with SMTP id w126mr6229311wmf.69.1489608954704; Wed, 15 Mar 2017 13:15:54 -0700 (PDT) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id 73sm1681918wml.19.2017.03.15.13.15.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Mar 2017 13:15:54 -0700 (PDT) From: Thomas Monjalon To: "Dumitrescu, Cristian" Cc: "De Lara Guarch, Pablo" , "Singh, Jasvinder" , dev@dpdk.org, "Doherty, Declan" Date: Wed, 15 Mar 2017 21:15:52 +0100 Message-ID: <9340287.qWvXv5QBAa@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D89126527609A4@IRSMSX108.ger.corp.intel.com> References: <1487969657-172541-2-git-send-email-jasvinder.singh@intel.com> <6450918.dWRLMxV3EZ@xps13> <3EB4FA525960D640B5BDFFD6A3D89126527609A4@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 1/2] librte_net: add crc init and compute APIs 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: Wed, 15 Mar 2017 20:15:55 -0000 2017-03-15 19:03, Dumitrescu, Cristian: > ... > > > > > > > I think it should be in librte_hash. > > > > > > > > > > > > Please check lib/librte_hash/rte_hash_crc.h > > > > > > > > > > Is it good to include payload crc calculation in hash library as I see all > > hash > > > > related functionality there? > > > > > > > > I think yes. Pablo? > > > > > > I think this doesn't belong in the hash library. These new functions calculate > > CRC, but not as a hash function. > > > > Can't we say that a CRC is a hash? What is a hash? > > A function generating the same output bytes from given input bytes. > > > > I think you must separate hash functions and hash table management. > > > > The fact that CRC32 instruction is opportunistically used to compute a hash digest/signature for load balancing (affinity-based) or hash tables (flow tables, ARP cache, etc) does not mean that all the code that uses CRC32 instruction should be placed in librte_hash. > > The purpose of the hash functions in librte_hash is to compute a digest/signature for a given array of bytes (the key) as fast as possible. Any hash function that produces a hash signature with good uniform distribution in a small amount of cycles belongs here, including those opportunistically using specialized CPU instructions such as CRC32 (or XOR, AESNI, etc). > > The code proposed in this patch is used to compute packet fields for various protocols that have a CRC field, such as FCS of Ethernet frames, etc. according to the relevant standard (IEEE 802, others). It is an utility to be used for implementing protocol-level functionality for various protocols from the network stack, similar to e.g. IP or UDP checksum. If we were to add an IP or UDCP checksum calculator, would you put it in librte_hash? > > The code from this patch is never going to be used to compute a fast signature/digest. Typically this CRC is computed over the entire frame/packet rather than just selected fields from the packet representing the application-specific flow key. Also note that the signature produced by CRC32 hash function from librte_hash is actually not the correct Cyclic Redundancy Check of that array of bytes (or, for math guys, of the associated polynomial), it is just a partial/intermediate value. > > Therefore, I suggest placing this code into: librte_ether (given that it can be used to compute Ethernet FCS), or librte_net, or librte_crc. Definitely not in librte_hash. I agree with you Cristian that the protocol layer must be in librte_net. But I think most of this patch is not protocol level. I think you agree with me that the code computing a "digest/signature for a given array of bytes" must go to librte_hash?