diff mbox

[dpdk-dev,v5,1/5] i40e: Use constant random hash keys

Message ID 1413861289-26662-2-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Zhang, Helin Oct. 21, 2014, 3:14 a.m. UTC 
  To be simpler, and remove the race condition, it uses prepared
constant random hash keys to replace runtime generating the hash
keys.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon Nov. 3, 2014, 7:49 a.m. UTC | #1 
Hi Helin,

The title is a bit surprising:
- it should be about RSS
- a constant cannot be really random ;)

2014-10-21 11:14, Helin Zhang:
> To be simpler, and remove the race condition, it uses prepared
> constant random hash keys to replace runtime generating the hash
> keys.

Could you explain what is the role of rss_key_default?

Thanks
Zhang, Helin Nov. 3, 2014, 8:18 a.m. UTC | #2 
Hi Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, November 3, 2014 3:50 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/5] i40e: Use constant random hash keys
> 
> Hi Helin,
> 
> The title is a bit surprising:
> - it should be about RSS
RSS makes use of hash function to route received packets, though hash function can be used for other cases, e.g. Flow director.

> - a constant cannot be really random ;)
The hash keys are generated by libc random function. It is preparatory to avoid calling random function for each port.

> 
> 2014-10-21 11:14, Helin Zhang:
> > To be simpler, and remove the race condition, it uses prepared
> > constant random hash keys to replace runtime generating the hash keys.
> 
> Could you explain what is the role of rss_key_default?
Hash function needs to be configured with keys, before end users configured them with specific keys, we need to provide a default keys which is generated by libc random function.
The random keys can get the hash function to route the received packets to all the queues well-proportioned.

> 
> Thanks
> --
> Thomas

Regards,
Helin
Thomas Monjalon Nov. 3, 2014, 8:59 a.m. UTC | #3 
2014-11-03 08:18, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > The title is a bit surprising:
> > - it should be about RSS
> 
> RSS makes use of hash function to route received packets, though
> hash function can be used for other cases, e.g. Flow director.

Yes but this patch is only changing rss_key_default so I guess it's
only related to RSS, right?

> > - a constant cannot be really random ;)
> 
> The hash keys are generated by libc random function.
> It is preparatory to avoid calling random function for each port.

Here, you remove the call to rte_rand by a constant value.

> > 2014-10-21 11:14, Helin Zhang:
> > > To be simpler, and remove the race condition, it uses prepared
> > > constant random hash keys to replace runtime generating the hash keys.
> > 
> > Could you explain what is the role of rss_key_default?
> 
> Hash function needs to be configured with keys, before end users configured
> them with specific keys, we need to provide a default keys which is
> generated by libc random function.
> The random keys can get the hash function to route the received packets to
> all the queues well-proportioned.
Zhang, Helin Nov. 6, 2014, 7:52 a.m. UTC | #4 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, November 3, 2014 4:59 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/5] i40e: Use constant random hash keys
> 
> 2014-11-03 08:18, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > The title is a bit surprising:
> > > - it should be about RSS
> >
> > RSS makes use of hash function to route received packets, though hash
> > function can be used for other cases, e.g. Flow director.
> 
> Yes but this patch is only changing rss_key_default so I guess it's only related to
> RSS, right?
Yes, it is related to RSS.

> 
> > > - a constant cannot be really random ;)
> >
> > The hash keys are generated by libc random function.
> > It is preparatory to avoid calling random function for each port.
> 
> Here, you remove the call to rte_rand by a constant value.
Yes, actually the hardware just needs random keys by default, no matter what the
real data it is. So array of random data would be better, as no need of cpu cycles,
and it is safer in multi-threads environments.

> 
> > > 2014-10-21 11:14, Helin Zhang:
> > > > To be simpler, and remove the race condition, it uses prepared
> > > > constant random hash keys to replace runtime generating the hash keys.
> > >
> > > Could you explain what is the role of rss_key_default?
> >
> > Hash function needs to be configured with keys, before end users
> > configured them with specific keys, we need to provide a default keys
> > which is generated by libc random function.
> > The random keys can get the hash function to route the received
> > packets to all the queues well-proportioned.

Regards,
Helin
Zhang, Helin Nov. 11, 2014, 3:30 a.m. UTC | #5 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, November 3, 2014 4:59 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 1/5] i40e: Use constant random hash keys
> 
> 2014-11-03 08:18, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > The title is a bit surprising:
> > > - it should be about RSS
> >
> > RSS makes use of hash function to route received packets, though hash
> > function can be used for other cases, e.g. Flow director.
> 
> Yes but this patch is only changing rss_key_default so I guess it's only related to
> RSS, right?
Yes, it is currently for rss only.

> 
> > > - a constant cannot be really random ;)
The comments could be re-worded.

> >
> > The hash keys are generated by libc random function.
> > It is preparatory to avoid calling random function for each port.
> 
> Here, you remove the call to rte_rand by a constant value.
No need to calculate it every time, like what Linux i40e driver does.

> 
> > > 2014-10-21 11:14, Helin Zhang:
> > > > To be simpler, and remove the race condition, it uses prepared
> > > > constant random hash keys to replace runtime generating the hash keys.
> > >
> > > Could you explain what is the role of rss_key_default?
> >
> > Hash function needs to be configured with keys, before end users
> > configured them with specific keys, we need to provide a default keys
> > which is generated by libc random function.
> > The random keys can get the hash function to route the received
> > packets to all the queues well-proportioned.

Regards,
Helin
diff mbox

Patch

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 3b75f0f..5e5cfbe 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -191,9 +191,6 @@  static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				enum rte_filter_op filter_op,
 				void *arg);
 
-/* Default hash key buffer for RSS */
-static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1];
-
 static struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
@@ -4117,9 +4114,12 @@  i40e_pf_config_rss(struct i40e_pf *pf)
 	}
 	if (rss_conf.rss_key == NULL || rss_conf.rss_key_len <
 		(I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t)) {
-		/* Calculate the default hash key */
-		for (i = 0; i <= I40E_PFQF_HKEY_MAX_INDEX; i++)
-			rss_key_default[i] = (uint32_t)rte_rand();
+		/* Random default keys */
+		static uint32_t rss_key_default[] = {0x6b793944,
+			0x23504cb5, 0x5bea75b6, 0x309f4f12, 0x3dc0a2b8,
+			0x024ddcdf, 0x339b8ca0, 0x4c4af64a, 0x34fac605,
+			0x55d85839, 0x3a58997d, 0x2ec938e1, 0x66031581};
+
 		rss_conf.rss_key = (uint8_t *)rss_key_default;
 		rss_conf.rss_key_len = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
 							sizeof(uint32_t);