| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Nov 2014 14:27:46 +0200
From: Or Gerlitz <ogerlitz@...lanox.com>
To: Eyal Perry <eyalpe@...lanox.com>
CC: Amir Vadai <amirv@...lanox.com>,
"David S. Miller" <davem@...emloft.net>,
Ben Hutchings <ben@...adent.org.uk>, <netdev@...r.kernel.org>,
"Yevgeny Petrilin" <yevgenyp@...lanox.com>
Subject: Re: [PATCH net-next 2/2] net/mlx4_en: Support for configurable RSS
hash function
On 11/5/2014 1:59 PM, Amir Vadai wrote:
> From: Eyal Perry <eyalpe@...lanox.com>
>
> The ConnectX HW is capable of using one of the following hash functions:
> Toeplitz and an XOR hash function.
> This patch implements ethtool_ops {get,set}_rxfh_func callbacks for the
> mlx4_en driver in order to query and configure the RSS hash function to
> be used by the device.
>
> Signed-off-by: Eyal Perry <eyalpe@...lanox.com>
> Signed-off-by: Amir Vadai <amirv@...lanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 40 +++++++++++++++++++++++++
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 9 ++++++
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 11 ++++---
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
> 4 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> index 8ea4d5b..f33785a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> @@ -973,6 +973,44 @@ static u32 mlx4_en_get_rxfh_indir_size(struct net_device *dev)
> return priv->rx_ring_num;
> }
>
> +static u32 mlx4_en_get_rxfh_func(struct net_device *dev)
> +{
> + struct mlx4_en_priv *priv = netdev_priv(dev);
> +
> + return priv->rss_hash_fn;
> +}
> +
> +static int mlx4_en_set_rxfh_func(struct net_device *dev, u32 hfunc)
> +{
> + struct mlx4_en_priv *priv = netdev_priv(dev);
> + struct mlx4_en_dev *mdev = priv->mdev;
> + u32 prev_rss_hash_fn = priv->rss_hash_fn;
> + int err = 0;
> +
> + if (!(hfunc & priv->rss_hash_fn_caps))
> + return -EINVAL;
> +
> + priv->rss_hash_fn = hfunc;
Seems that you are blindly setting here priv->rss_hash_fn to the
function (xor or top) requested
by the user w.o prior checking if the firmware actually supports that
(e.g test it against priv->rss_hash_fn_caps, right?
> + if (hfunc == RSS_HASH_TOP && !(dev->features & NETIF_F_RXHASH))
> + en_warn(priv,
> + "Toeplitz hash function should be used in conjunction with RX hashing for optimal performance\n");
> + if (hfunc == RSS_HASH_XOR && (dev->features & NETIF_F_RXHASH))
> + en_warn(priv,
> + "Enabling both XOR Hash function and RX Hashing can limit RPS functionality\n");
> +
> + mutex_lock(&mdev->state_lock);
> + if (priv->port_up && priv->rss_hash_fn != prev_rss_hash_fn) {
> + mlx4_en_stop_port(dev, 1);
> + err = mlx4_en_start_port(dev);
> + }
> + mutex_unlock(&mdev->state_lock);
> +
> + if (err)
> + en_err(priv, "Failed to restart port %d\n", priv->port);
> +
> + return err;
> +}
> +
> static int mlx4_en_get_rxfh(struct net_device *dev, u32 *ring_index, u8 *key)
> {
> struct mlx4_en_priv *priv = netdev_priv(dev);
> @@ -1799,6 +1837,8 @@ const struct ethtool_ops mlx4_en_ethtool_ops = {
> .get_rxnfc = mlx4_en_get_rxnfc,
> .set_rxnfc = mlx4_en_set_rxnfc,
> .get_rxfh_indir_size = mlx4_en_get_rxfh_indir_size,
> + .get_rxfh_func = mlx4_en_get_rxfh_func,
> + .set_rxfh_func = mlx4_en_set_rxfh_func,
> .get_rxfh = mlx4_en_get_rxfh,
> .set_rxfh = mlx4_en_set_rxfh,
> .get_channels = mlx4_en_get_channels,
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 0efbae9..a9e96a6 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2585,6 +2585,15 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> dev->features |= NETIF_F_GSO_UDP_TUNNEL;
> }
>
> + if (mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RSS_XOR) {
> + priv->rss_hash_fn_caps |= RSS_HASH_XOR;
> + priv->rss_hash_fn = RSS_HASH_XOR;
> + }
> + if (mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RSS_TOP) {
> + priv->rss_hash_fn_caps |= RSS_HASH_TOP;
> + priv->rss_hash_fn = RSS_HASH_TOP;
> + }
> +
> mdev->pndev[port] = dev;
>
> netif_carrier_off(dev);
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index b173a0c..41c1ff9 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -1113,10 +1113,13 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
> }
>
> rss_context->flags = rss_mask;
> - rss_context->hash_fn = MLX4_RSS_HASH_TOP;
> - for (i = 0; i < 10; i++)
> - rss_context->rss_key[i] = cpu_to_be32(rsskey[i]);
> -
> + if (priv->rss_hash_fn & MLX4_RSS_HASH_XOR) {
> + rss_context->hash_fn = MLX4_RSS_HASH_XOR;
> + } else {
> + rss_context->hash_fn = MLX4_RSS_HASH_TOP;
> + for (i = 0; i < 10; i++)
> + rss_context->rss_key[i] = cpu_to_be32(rsskey[i]);
> + }
So this patch effectively changes the default from top to xor, right? is
that intentional? if yes, spare few words on the change log.
> err = mlx4_qp_to_ready(mdev->dev, &priv->res.mtt, &context,
> &rss_map->indir_qp, &rss_map->indir_state);
> if (err)
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index ef83d12..e1c3364 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -375,7 +375,6 @@ struct mlx4_en_port_profile {
> };
>
> struct mlx4_en_profile {
> - int rss_xor;
> int udp_rss;
> u8 rss_mask;
> u32 active_ports;
> @@ -615,6 +614,8 @@ struct mlx4_en_priv {
> __be16 vxlan_port;
>
> u32 pflags;
> + u8 rss_hash_fn;
> + u8 rss_hash_fn_caps;
> };
>
> enum mlx4_en_wol {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists