| Message ID | 153571495680.372632.9464993927924201878.stgit@buzz |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [RFC] net/mlx5_en: switch to Toeplitz RSS hash by default | expand |
On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote: > XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits. > It seems not enough for RFS. All other drivers use toeplitz. > > Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used > together with NETIF_F_RXHASH (enabled by default too): "Enabling both > XOR Hash function and RX Hashing can limit RPS functionality". > > XOR is default in mlx5_en since commit 2be6967cdbc9 > ("net/mlx5e: Support ETH_RSS_HASH_XOR"). > > Hash function could be set via ethtool. But it would be nice to have > single standard for drivers or proper description why this one is special. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > --- Hi Konstantin, Thanks for the patch. I understand the motivation. This change affects the default out-of-the-box behavior and requires a full performance cycle. We'll run performance regression tomorrow, results should be ready by EOW. I'll update. Regards, Tariq
On 02.09.2018 12:29, Tariq Toukan wrote: > > > On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote: >> XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits. >> It seems not enough for RFS. All other drivers use toeplitz. >> >> Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used >> together with NETIF_F_RXHASH (enabled by default too): "Enabling both >> XOR Hash function and RX Hashing can limit RPS functionality". >> >> XOR is default in mlx5_en since commit 2be6967cdbc9 >> ("net/mlx5e: Support ETH_RSS_HASH_XOR"). >> >> Hash function could be set via ethtool. But it would be nice to have >> single standard for drivers or proper description why this one is special. >> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >> --- > > Hi Konstantin, > > Thanks for the patch. > > I understand the motivation. > > This change affects the default out-of-the-box behavior and requires a full performance cycle. We'll run performance regression tomorrow, > results should be ready by EOW. > > I'll update. Ok, thank you. The only mention I've found in your documentation http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Linux_User_Manual_v4_4.pdf is --- 1.1.10 RSS Support 1.1.10.1 RSS Hash Function The device has the ability to use XOR as the RSS distribution function, instead of the default Toplitz function. The XOR function can be better distributed among driver's receive queues in small number of streams, where it distributes each TCP/UDP stream to a different queue. --- So Toeplitz is supposed to be default hash function for all versions of drivers and hardware. Also XOR8 seems vulnerable for ddos - hash is predictable, no random\secret vector, only 8 bits. So, it's easy to route all flows into one point. As we got it by accident. Moreover, in kernel 4.4.y hash switch via ethtool is broken and does not work =) > > Regards, > Tariq
On Sun, Sep 2, 2018 at 2:55 AM, Konstantin Khlebnikov <khlebnikov@yandex-team.ru> wrote: > On 02.09.2018 12:29, Tariq Toukan wrote: >> >> >> >> On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote: >>> >>> XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits. >>> It seems not enough for RFS. All other drivers use toeplitz. >>> >>> Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used >>> together with NETIF_F_RXHASH (enabled by default too): "Enabling both >>> XOR Hash function and RX Hashing can limit RPS functionality". >>> >>> XOR is default in mlx5_en since commit 2be6967cdbc9 >>> ("net/mlx5e: Support ETH_RSS_HASH_XOR"). >>> >>> Hash function could be set via ethtool. But it would be nice to have >>> single standard for drivers or proper description why this one is >>> special. >>> >>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >>> --- >> >> >> Hi Konstantin, >> >> Thanks for the patch. >> >> I understand the motivation. >> >> This change affects the default out-of-the-box behavior and requires a >> full performance cycle. We'll run performance regression tomorrow, results >> should be ready by EOW. >> > I'll update. > > > Ok, thank you. > > The only mention I've found in your documentation > http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Linux_User_Manual_v4_4.pdf > > is > --- > 1.1.10 RSS Support > 1.1.10.1 RSS Hash Function > The device has the ability to use XOR as the RSS distribution function, > instead of the default > Toplitz function. > The XOR function can be better distributed among driver's receive queues in > small number of > streams, where it distributes each TCP/UDP stream to a different queue. > --- > > So Toeplitz is supposed to be default hash function for all versions of > drivers and hardware. > > Also XOR8 seems vulnerable for ddos - hash is predictable, no random\secret > vector, only 8 bits. > So, it's easy to route all flows into one point. As we got it by accident. > > Moreover, in kernel 4.4.y hash switch via ethtool is broken and does not > work =) > is it broken in mlx5 only or for the whole kernel ? If it is mlx5 then this might be the reason: commit 2d75b2bc8a8c0ce5567a6ecef52e194d117efe3f net/mlx5e: Add ethtool RSS configuration options was submitted to kernel 4.3 and an important fix for hash function change was submitted to 4.5: commit bdfc028de1b3cd59490d5413a5c87b0fa50040c2 Author: Tariq Toukan <tariqt@mellanox.com> Date: Mon Feb 29 21:17:12 2016 +0200 net/mlx5e: Fix ethtool RX hash func configuration change We should modify TIRs explicitly to apply the new RSS configuration. The light ndo close/open calls do not "refresh" them. Fixes: 2d75b2bc8a8c ('net/mlx5e: Add ethtool RSS configuration options') >> >> Regards, >> Tariq
On 06.09.2018 08:24, Saeed Mahameed wrote: > On Sun, Sep 2, 2018 at 2:55 AM, Konstantin Khlebnikov > <khlebnikov@yandex-team.ru> wrote: >> On 02.09.2018 12:29, Tariq Toukan wrote: >>> >>> >>> >>> On 31/08/2018 2:29 PM, Konstantin Khlebnikov wrote: >>>> >>>> XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits. >>>> It seems not enough for RFS. All other drivers use toeplitz. >>>> >>>> Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used >>>> together with NETIF_F_RXHASH (enabled by default too): "Enabling both >>>> XOR Hash function and RX Hashing can limit RPS functionality". >>>> >>>> XOR is default in mlx5_en since commit 2be6967cdbc9 >>>> ("net/mlx5e: Support ETH_RSS_HASH_XOR"). >>>> >>>> Hash function could be set via ethtool. But it would be nice to have >>>> single standard for drivers or proper description why this one is >>>> special. >>>> >>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >>>> --- >>> >>> >>> Hi Konstantin, >>> >>> Thanks for the patch. >>> >>> I understand the motivation. >>> >>> This change affects the default out-of-the-box behavior and requires a >>> full performance cycle. We'll run performance regression tomorrow, results >>> should be ready by EOW. >>> > I'll update. >> >> >> Ok, thank you. >> >> The only mention I've found in your documentation >> http://www.mellanox.com/related-docs/prod_software/Mellanox_EN_for_Linux_User_Manual_v4_4.pdf >> >> is >> --- >> 1.1.10 RSS Support >> 1.1.10.1 RSS Hash Function >> The device has the ability to use XOR as the RSS distribution function, >> instead of the default >> Toplitz function. >> The XOR function can be better distributed among driver's receive queues in >> small number of >> streams, where it distributes each TCP/UDP stream to a different queue. >> --- >> >> So Toeplitz is supposed to be default hash function for all versions of >> drivers and hardware. >> >> Also XOR8 seems vulnerable for ddos - hash is predictable, no random\secret >> vector, only 8 bits. >> So, it's easy to route all flows into one point. As we got it by accident. >> >> Moreover, in kernel 4.4.y hash switch via ethtool is broken and does not >> work =) >> > > is it broken in mlx5 only or for the whole kernel ? In mlx5 only - interface works but makes no effect. For now we have disabled RXHASH as workaround. > > If it is mlx5 then this might be the reason: > commit 2d75b2bc8a8c0ce5567a6ecef52e194d117efe3f > net/mlx5e: Add ethtool RSS configuration options > > was submitted to kernel 4.3 > > and an important fix for hash function change was submitted to 4.5: > > commit bdfc028de1b3cd59490d5413a5c87b0fa50040c2 > Author: Tariq Toukan <tariqt@mellanox.com> > Date: Mon Feb 29 21:17:12 2016 +0200 > > net/mlx5e: Fix ethtool RX hash func configuration change > > We should modify TIRs explicitly to apply the new RSS configuration. > The light ndo close/open calls do not "refresh" them. > > Fixes: 2d75b2bc8a8c ('net/mlx5e: Add ethtool RSS configuration options') > I think so, but this patch also seems flawed and fixed in commit a100ff3eef193d2d79daf98dcd97a54776ffeb78 Author: Gal Pressman <galp@mellanox.com> Date: Thu Jan 12 16:25:46 2017 +0200 net/mlx5e: Fix update of hash function/key via ethtool Modifying TIR hash should change selected fields bitmask in addition to the function and key. And maybe more, there a lot of progress since v4.4 > >>> >>> Regards, >>> Tariq
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 5a7939e70190..def9fb5dcbff 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -4558,7 +4558,7 @@ void mlx5e_build_nic_params(struct mlx5_core_dev *mdev, params->tx_min_inline_mode = mlx5e_params_calculate_tx_min_inline(mdev); /* RSS */ - params->rss_hfunc = ETH_RSS_HASH_XOR; + params->rss_hfunc = ETH_RSS_HASH_TOP; netdev_rss_key_fill(params->toeplitz_hash_key, sizeof(params->toeplitz_hash_key)); mlx5e_build_default_indir_rqt(params->indirection_rqt, MLX5E_INDIR_RQT_SIZE, max_channels);
XOR (MLX5_RX_HASH_FN_INVERTED_XOR8) gives only 8 bits. It seems not enough for RFS. All other drivers use toeplitz. Driver mlx4_en uses Toeplitz by default and warns if hash XOR is used together with NETIF_F_RXHASH (enabled by default too): "Enabling both XOR Hash function and RX Hashing can limit RPS functionality". XOR is default in mlx5_en since commit 2be6967cdbc9 ("net/mlx5e: Support ETH_RSS_HASH_XOR"). Hash function could be set via ethtool. But it would be nice to have single standard for drivers or proper description why this one is special. Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)