[dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
On Fri, Jun 19, 2015 at 07:14:15PM +0300, Vladimir Medvedkin wrote:
> Hi Bruce,
>
> 2015-06-19 18:59 GMT+03:00 Richardson, Bruce :
>
> >
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladimir Medvedkin
> > > Sent: Friday, June 19, 2015 3:56 PM
> > > To: dev at dpdk.org
> > > Subject: [dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
> > >
> > > v4 changes
> > > - Fix copyright
> > > - rename bswap_mask constant, add rte_ prefix
> > > - change rte_ipv[46]_tuple struct
> > > - change rte_thash_load_v6_addr prototype
> > >
> > > v3 changes
> > > - Rework API to be more generic
> > > - Add sctp_tag into tuple
> > >
> > > v2 changes
> > > - Add ipv6 support
> > > - Various style fixes
> > >
> >
> > Missing signoff line.
> >
> > > ---
> > > lib/librte_hash/Makefile| 1 +
> > > lib/librte_hash/rte_thash.h | 202
> > >
> > > 2 files changed, 203 insertions(+)
> > > create mode 100644 lib/librte_hash/rte_thash.h
> > >
> > <...snip...>
> > > +
> > > +/* Byte swap mask used for converting IPv6 address 4-byte chunks to CPU
> > > byte order */
> > > +static const __m128i rte_thash_ipv6_bswap_mask = {0x0405060700010203,
> > > 0x0C0D0E0F08090A0B};
> > > +
> > > +#define RTE_THASH_V4_L3 2 /*calculate hash of ipv4 header
> > only*/
> > > +#define RTE_THASH_V4_L4 3 /*calculate hash of ipv4 +
> > transport
> > > headers*/
> > > +#define RTE_THASH_V6_L3 8 /*calculate hash of ipv6 header
> > only
> > > */
> > > +#define RTE_THASH_V6_L4 9 /*calculate hash of ipv6 +
> > transport
> > > headers */
> >
> > I'm still not seeing why these values need to be defined here, rather than
> > in a specific app.
> > Also, the choice of values for these defines seems strange to me? How were
> > they chosen?
> >
> This is a predefined values. They mean the length (in 4-bytes) of the
> input data
> in hashing. I think it's like defines in rte_ip.h, for example.
>
I'm still not convined of the need for them in this file. If they are to be
kept though, they should instead have "LEN" in the name to indicate that they
are the lengths of the different keys. You could also make this clearer by
changing the L4 values to be computed using "sizeof()" given that the
appropriate
tuple structures are present later in the header file to allow the sizes to be
correctly calculated.
/Bruce
[dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
Hi Bruce,
2015-06-19 18:59 GMT+03:00 Richardson, Bruce :
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladimir Medvedkin
> > Sent: Friday, June 19, 2015 3:56 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
> >
> > v4 changes
> > - Fix copyright
> > - rename bswap_mask constant, add rte_ prefix
> > - change rte_ipv[46]_tuple struct
> > - change rte_thash_load_v6_addr prototype
> >
> > v3 changes
> > - Rework API to be more generic
> > - Add sctp_tag into tuple
> >
> > v2 changes
> > - Add ipv6 support
> > - Various style fixes
> >
>
> Missing signoff line.
>
> > ---
> > lib/librte_hash/Makefile| 1 +
> > lib/librte_hash/rte_thash.h | 202
> >
> > 2 files changed, 203 insertions(+)
> > create mode 100644 lib/librte_hash/rte_thash.h
> >
> <...snip...>
> > +
> > +/* Byte swap mask used for converting IPv6 address 4-byte chunks to CPU
> > byte order */
> > +static const __m128i rte_thash_ipv6_bswap_mask = {0x0405060700010203,
> > 0x0C0D0E0F08090A0B};
> > +
> > +#define RTE_THASH_V4_L3 2 /*calculate hash of ipv4 header
> only*/
> > +#define RTE_THASH_V4_L4 3 /*calculate hash of ipv4 +
> transport
> > headers*/
> > +#define RTE_THASH_V6_L3 8 /*calculate hash of ipv6 header
> only
> > */
> > +#define RTE_THASH_V6_L4 9 /*calculate hash of ipv6 +
> transport
> > headers */
>
> I'm still not seeing why these values need to be defined here, rather than
> in a specific app.
> Also, the choice of values for these defines seems strange to me? How were
> they chosen?
>
This is a predefined values. They mean the length (in 4-bytes) of the
input data
in hashing. I think it's like defines in rte_ip.h, for example.
>
> /Bruce
>
>
[dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Richardson, Bruce > Sent: Friday, June 19, 2015 5:00 PM > To: Vladimir Medvedkin; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS > > Missing signoff line. > Hi, It would also be worth running Linux checkpatch against the patch, which would have caught the missing signoff. It also shows a lot of warnings in the patch which indicates conflicts with the coding guidelines. John
[dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vladimir Medvedkin
> Sent: Friday, June 19, 2015 3:56 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
>
> v4 changes
> - Fix copyright
> - rename bswap_mask constant, add rte_ prefix
> - change rte_ipv[46]_tuple struct
> - change rte_thash_load_v6_addr prototype
>
> v3 changes
> - Rework API to be more generic
> - Add sctp_tag into tuple
>
> v2 changes
> - Add ipv6 support
> - Various style fixes
>
Missing signoff line.
> ---
> lib/librte_hash/Makefile| 1 +
> lib/librte_hash/rte_thash.h | 202
>
> 2 files changed, 203 insertions(+)
> create mode 100644 lib/librte_hash/rte_thash.h
>
<...snip...>
> +
> +/* Byte swap mask used for converting IPv6 address 4-byte chunks to CPU
> byte order */
> +static const __m128i rte_thash_ipv6_bswap_mask = {0x0405060700010203,
> 0x0C0D0E0F08090A0B};
> +
> +#define RTE_THASH_V4_L3 2 /*calculate hash of ipv4 header only*/
> +#define RTE_THASH_V4_L4 3 /*calculate hash of ipv4 + transport
> headers*/
> +#define RTE_THASH_V6_L3 8 /*calculate hash of ipv6 header only
> */
> +#define RTE_THASH_V6_L4 9 /*calculate hash of ipv6 + transport
> headers */
I'm still not seeing why these values need to be defined here, rather than in a
specific app.
Also, the choice of values for these defines seems strange to me? How were they
chosen?
/Bruce
[dpdk-dev] [PATCH v4] Add toeplitz hash algorithm used by RSS
v4 changes
- Fix copyright
- rename bswap_mask constant, add rte_ prefix
- change rte_ipv[46]_tuple struct
- change rte_thash_load_v6_addr prototype
v3 changes
- Rework API to be more generic
- Add sctp_tag into tuple
v2 changes
- Add ipv6 support
- Various style fixes
---
lib/librte_hash/Makefile| 1 +
lib/librte_hash/rte_thash.h | 202
2 files changed, 203 insertions(+)
create mode 100644 lib/librte_hash/rte_thash.h
diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile
index 3696cb1..981230b 100644
--- a/lib/librte_hash/Makefile
+++ b/lib/librte_hash/Makefile
@@ -49,6 +49,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += rte_fbk_hash.c
SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include := rte_hash.h
SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_hash_crc.h
SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_jhash.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_thash.h
SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_fbk_hash.h
# this lib needs eal
diff --git a/lib/librte_hash/rte_thash.h b/lib/librte_hash/rte_thash.h
new file mode 100644
index 000..7b3cc52
--- /dev/null
+++ b/lib/librte_hash/rte_thash.h
@@ -0,0 +1,202 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Vladimir Medvedkin
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_THASH_H
+#define _RTE_THASH_H
+
+/**
+ * @file
+ *
+ * toeplitz hash functions.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * 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)
+ */
+
+#include
+#include
+#include
+#include
+
+/* Byte swap mask used for converting IPv6 address 4-byte chunks to CPU byte
order */
+static const __m128i rte_thash_ipv6_bswap_mask = {0x0405060700010203,
0x0C0D0E0F08090A0B};
+
+#define RTE_THASH_V4_L3 2 /*calculate hash of ipv4 header only*/
+#define RTE_THASH_V4_L4 3 /*calculate hash of ipv4 + transport
headers*/
+#define RTE_THASH_V6_L3 8 /*calculate hash of ipv6 header only */
+#define RTE_THASH_V6_L4 9 /*calculate hash of ipv6 + transport
headers */
+
+/**
+ * IPv4 tuple
+ * addreses and ports/sctp_tag have to be CPU byte order
+ */
+struct rte_ipv4_tuple {
+ uint32_tsrc_addr;
+ uint32_tdst_addr;
+ union {
+ struct {
+ uint16_t dport;
+ uint16_t sport;
+ };
+ uint32_tsctp_tag;
+ };
+};
+
+/**
+ * IPv6 tuple
+ * Addresses have to be filled by rte_thash_load_v6_addr()
+ * ports/sctp_tag have to be CPU byte order
+ */
+struct rte_ipv6_tuple {
+ uint8_t src_addr[16];
+ uint8_t dst_addr[16];
+ union {
+ struct {
+ uint16_t dport;
+ uint16_t sport;
+ };
+ uint32_tsctp_tag;
+ };
+};
+
+union rte_thash_tuple {
+ struct rte_ipv4_tuple v4;
+ struct rte_ipv6_tuple v6;
+} __attribute__((aligned(16)));
+
+/**
+ * Prepare special converted key to use with rte_softrss_be()
+ * @param orig
+ * pointer to original RSS key
+ * @param targ
+ * pointer to target RSS key
+ * @param len
+ * RSS
