From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f178.google.com (mail-lb0-f178.google.com [209.85.217.178]) by dpdk.org (Postfix) with ESMTP id D64335921 for ; Thu, 9 Apr 2015 14:50:23 +0200 (CEST) Received: by lbcga7 with SMTP id ga7so40560828lbc.1 for ; Thu, 09 Apr 2015 05:50:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=/L4dXvZf+M1Mj5Yv5E27jpKGgBKC+6jlYHTmhukwCkw=; b=YIgnBM5r+Ni1y1JzjBDsTq6n/k/OXb27fbLXhJ6jsLdmmL8jE0fzA/pndyHjIeKB+x fSSquueoJ4qdyiAhqAerOLphtHaDBpnKzRDl2X0Q+x21jCD9Lu4CJ7VtqLhTqRvyjMVb lzMj2aR5K1cfWPyEff7lwmR35He/MX8qa46C5UT3pTnpAD8eyl/VGJjHWmLAJBMc2G8k ZCwxlr26AEFIPbiaAbK+H3cyilmSZM02m8u6ScT6zm+pt3B+UCi1sOuHWVGOuRLGMaTQ 5j3B0KCfJGR7fLzhpNl3rrUycXKXEo82KUo/VlN1+tYTcpnR05YuN9MglnbGg1HL2TSN 5O4Q== MIME-Version: 1.0 X-Received: by 10.152.87.46 with SMTP id u14mr4282029laz.82.1428583823445; Thu, 09 Apr 2015 05:50:23 -0700 (PDT) Received: by 10.114.83.41 with HTTP; Thu, 9 Apr 2015 05:50:23 -0700 (PDT) In-Reply-To: <20150408152430.0bdedfd6@urahara> References: <1428519973-10550-1-git-send-email-medvedkinv@gmail.com> <20150408152430.0bdedfd6@urahara> Date: Thu, 9 Apr 2015 15:50:23 +0300 Message-ID: From: Vladimir Medvedkin To: Stephen Hemminger Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] Add toeplitz hash algorithm X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Apr 2015 12:50:24 -0000 Hi Stephen, 2015-04-09 1:24 GMT+03:00 Stephen Hemminger : > On Wed, 8 Apr 2015 15:06:13 -0400 > Vladimir Medvedkin wrote: > > > Software implementation of the Toeplitz hash function used by RSS. > > Can be used either for packet distribution on single queue NIC > > or for simulating of RSS computation on specific NIC (for example > > after GRE header decapsulating). > > > > Signed-off-by: Vladimir Medvedkin > > > +enum rte_thash_flag { > > + RTE_THASH_L3 = 0, //calculate hash tacking into account only > l3 header > > + RTE_THASH_L4 //calculate hash tacking into account l4 + > l4 headers > > +}; > > + > > +/** > > + * Prepare special converted key to use with rte_softrss_be() > > + * @param orig > > + * pointer to original RSS key > > + * @param targ > > + * pointer to target RSS key > > + */ > > + > > +static inline void > > +rte_convert_rss_key(uint32_t *orig, uint32_t *targ) > orig should be const > > > +{ > > + int i; > > + for (i = 0; i < 10; i++) { > > + targ[i] = rte_be_to_cpu_32(orig[i]); > > + } > > +} > > > +static inline uint32_t > > +rte_softrss(uint32_t sip, uint32_t dip, uint16_t sp, uint16_t dp, enum > rte_thash_flag flag, uint32_t *rss_key) > > rss_key should be const > > > +{ > > + uint32_t ret = 0; > > + int i; > > + for (i = 0; i < 32; i++) { > blank line after declaration please > > > + if (sip & (1 << (31 - i))) { > > + ret ^= (rte_cpu_to_be_32(*rss_key) << > i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i)); > > Long expression > 80 characters. > Repeated multiple times (should be inline) > Extra parens () > Thanks for remarks, I'll fix it. > Extension to 64 bits is only to avoid compiler warning? > No, in case when i = 0 we shift uint32_t left by 32 bits, which leads to undefined behaviour. In fact, shift counter just masked to 5 bits, so count range is limited to 0 to 31. > > > > + } > > + } > > + rss_key++; > > + for (i = 0; i < 32; i++) { > > + if (dip & (1 << (31 - i))) { > > + ret ^= (rte_cpu_to_be_32(*rss_key) << > i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i)); > > + } > > + } > > + if (flag == RTE_THASH_L4) { > > + rss_key++; > > + for (i = 0; i < 32; i++) { > > + if (((sp<<16)|dp) & (1 << (31 - i))) { > > + ret ^= (rte_cpu_to_be_32(*rss_key) << > i)|(uint32_t)((uint64_t)(rte_cpu_to_be_32(*(rss_key + 1))) >> (32 - i)); > > + } > > + } > > + } > > + return ret; > > +} > > + > > +/** > > + * Optimized implementation. > > + * If you want the calculated hash value matches NIC RSS value > > + * you have to use special converted key. > > + * All ip's and ports have to be CPU byte order. > > + * @param sip > > + * Source ip address. > > + * @param dip > > + * Destination ip address. > > + * @param sp > > + * Source TCP|UDP port. > > + * @param dp > > + * Destination TCP|UDP port. > > + * @param flag > > + * RTE_THASH_L3: calculate hash tacking into account only sip and > dip > > + * RTE_THASH_L4: calculate hash tacking into account sip, dip, sp > and dp > > + * @param *rss_key > > + * Pointer to 40-byte RSS hash key. > > + * @return > > + * Calculated hash value. > > + */ > > + > > +static inline uint32_t > > +rte_softrss_be(uint32_t sip, uint32_t dip, uint16_t sp, uint16_t dp, > enum rte_thash_flag flag, uint32_t *rss_key) > > +{ > > Same problems as previous code. > Also lots of copy paste (see Do Not Repeat Yourself principle). >