netdev - Re: [PATCH net-next 7/9] net/mlx4_en: Manage hash of MAC addresses per port

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 05 Feb 2013 05:47:43 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Amir Vadai <amirv@...lanox.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Yan Burman <yanb@...lanox.com>
Subject: Re: [PATCH net-next 7/9] net/mlx4_en: Manage hash of MAC addresses
 per port

On Tue, 2013-02-05 at 13:28 +0200, Amir Vadai wrote:
> From: Yan Burman <yanb@...lanox.com>
> 
> As a preparation step for supporting multiple unicast addresses, store MAC addresses in hash table.
> Remove the radix tree for MAC addresses per QP, as it's not in use.
> 
> Signed-off-by: Yan Burman <yanb@...lanox.com>
> Signed-off-by: Amir Vadai <amirv@...lanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   89 +++++++++++++++---------
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   23 +++++-
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |    6 ++-
>  3 files changed, 80 insertions(+), 38 deletions(-)
> 

...

>  
>  alloc_err:
>  	mlx4_en_uc_steer_release(priv, priv->dev->dev_addr, *qpn, reg_id);
> @@ -568,7 +565,6 @@ static void mlx4_en_put_qp(struct mlx4_en_priv *priv)
>  {
>  	struct mlx4_en_dev *mdev = priv->mdev;
>  	struct mlx4_dev *dev = mdev->dev;
> -	struct mlx4_mac_entry *entry;
>  	int qpn = priv->base_qpn;
>  	u64 mac = mlx4_en_mac_to_u64(priv->dev->dev_addr);
>  
> @@ -577,15 +573,27 @@ static void mlx4_en_put_qp(struct mlx4_en_priv *priv)
>  	mlx4_unregister_mac(dev, priv->port, mac);
>  
>  	if (dev->caps.steering_mode != MLX4_STEERING_MODE_A0) {
> -		entry = radix_tree_lookup(&priv->mac_tree, qpn);
> -		if (entry) {
> -			en_dbg(DRV, priv, "Releasing qp: port %d, MAC %pM, qpn %d\n",
> -			       priv->port, entry->mac, qpn);
> -			mlx4_en_uc_steer_release(priv, entry->mac,
> -						 qpn, entry->reg_id);
> -			mlx4_qp_release_range(dev, qpn, 1);
> -			radix_tree_delete(&priv->mac_tree, qpn);
> -			kfree(entry);
> +		struct mlx4_mac_entry *entry;
> +		struct hlist_node *n, *tmp;
> +		struct hlist_head *bucket;
> +		unsigned int mac_hash;
> +
> +		mac_hash = priv->dev->dev_addr[MLX4_EN_MAC_HASH_IDX];
> +		bucket = &priv->mac_hash[mac_hash];
> +		hlist_for_each_entry_safe(entry, n, tmp, bucket, hlist) {
> +			if (ether_addr_equal_64bits(entry->mac,
> +						    priv->dev->dev_addr)) {
> +				en_dbg(DRV, priv, "Releasing qp: port %d, MAC %pM, qpn %d\n",
> +				       priv->port, priv->dev->dev_addr, qpn);
> +				mlx4_en_uc_steer_release(priv, entry->mac,
> +							 qpn, entry->reg_id);
> +				mlx4_qp_release_range(dev, qpn, 1);
> +
> +				hlist_del_rcu(&entry->hlist);
> +				synchronize_rcu();

Please use kfree_rcu()

> +				kfree(entry);
> +				break;
> +			}
>  		}
>  	}
>  }
> @@ -595,26 +603,39 @@ static int mlx4_en_replace_mac(struct mlx4_en_priv *priv, int qpn,
>  {
>  	struct mlx4_en_dev *mdev = priv->mdev;
>  	struct mlx4_dev *dev = mdev->dev;
> -	struct mlx4_mac_entry *entry;
>  	int err = 0;
>  	u64 new_mac_u64 = mlx4_en_mac_to_u64(new_mac);
>  
>  	if (dev->caps.steering_mode != MLX4_STEERING_MODE_A0) {
> -		u64 prev_mac_u64;
> -
> -		entry = radix_tree_lookup(&priv->mac_tree, qpn);
> -		if (!entry)
> -			return -EINVAL;
> -		prev_mac_u64 = mlx4_en_mac_to_u64(entry->mac);
> -		mlx4_en_uc_steer_release(priv, entry->mac,
> -					 qpn, entry->reg_id);
> -		mlx4_unregister_mac(dev, priv->port, prev_mac_u64);
> -		memcpy(entry->mac, new_mac, ETH_ALEN);
> -		entry->reg_id = 0;
> -		mlx4_register_mac(dev, priv->port, new_mac_u64);
> -		err = mlx4_en_uc_steer_add(priv, new_mac,
> -					   &qpn, &entry->reg_id);
> -		return err;
> +		struct hlist_head *bucket;
> +		unsigned int mac_hash;
> +		struct mlx4_mac_entry *entry;
> +		struct hlist_node *n, *tmp;
> +		u64 prev_mac_u64 = mlx4_en_mac_to_u64(prev_mac);
> +
> +		bucket = &priv->mac_hash[prev_mac[MLX4_EN_MAC_HASH_IDX]];
> +		hlist_for_each_entry_safe(entry, n, tmp, bucket, hlist) {
> +			if (ether_addr_equal_64bits(entry->mac, prev_mac)) {
> +				mlx4_en_uc_steer_release(priv, entry->mac,
> +							 qpn, entry->reg_id);
> +				mlx4_unregister_mac(dev, priv->port,
> +						    prev_mac_u64);
> +				hlist_del_rcu(&entry->hlist);
> +				synchronize_rcu();
> +				memcpy(entry->mac, new_mac, ETH_ALEN);
> +				entry->reg_id = 0;
> +				mac_hash = new_mac[MLX4_EN_MAC_HASH_IDX];
> +				hlist_add_head_rcu(&entry->hlist,
> +						   &priv->mac_hash[mac_hash]);
> +				synchronize_rcu();

Why is this synchronize_rcu() is needed ?

> +				mlx4_register_mac(dev, priv->port, new_mac_u64);
> +				err = mlx4_en_uc_steer_add(priv, new_mac,
> +							   &qpn,
> +							   &entry->reg_id);
> +				return err;
> +			}
> +		}
> +		return -EINVAL;
>  	}


--
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