| 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
| ||
|
Message-ID: <56534B8F.7040006@gmail.com>
Date: Mon, 23 Nov 2015 09:23:27 -0800
From: Alexander Duyck <alexander.duyck@...il.com>
To: Or Gerlitz <ogerlitz@...lanox.com>,
"David S. Miller" <davem@...emloft.net>
Cc: netdev@...r.kernel.org, Don Dutile <ddutile@...hat.com>,
Doug Ledford <dledford@...hat.com>,
Saeed Mahameed <saeedm@...lanox.com>,
Tal Alon <talal@...lanox.com>,
Hadar Har-Zion <hadarh@...lanox.com>,
Rony Efraim <ronye@...lanox.com>
Subject: Re: [PATCH net-next 09/18] net/mlx5e: Write UC/MC list and promisc
mode into vport context
On 11/23/2015 03:11 AM, Or Gerlitz wrote:
> From: Saeed Mahameed <saeedm@...lanox.com>
>
> Each Vport/vNIC must notify underlying e-Switch layer
> for UC/MC list and promisc mode updates, in-order to update
> l2 tables and SR-IOV FDB tables.
>
> We do that at set_rx_mode ndo.
>
> preperation for ethernet-SRIOV and l2 table management.
>
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@...lanox.com>
> ---
> .../ethernet/mellanox/mlx5/core/en_flow_table.c | 99 ++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_flow_table.c b/drivers/net/ethernet/mellanox/mlx5/core/en_flow_table.c
> index 22d603f..9a021be 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_flow_table.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_flow_table.c
> @@ -671,6 +671,103 @@ static void mlx5e_sync_netdev_addr(struct mlx5e_priv *priv)
> netif_addr_unlock_bh(netdev);
> }
>
> +/* Returns a pointer to an array of type u8[][ETH_ALEN] */
> +static u8 (*mlx5e_build_addr_array(struct mlx5e_priv *priv, int list_type,
> + int *size))[ETH_ALEN]
This is just ugly. Isn't there a way you can just return a u8 pointer
and assume the ETH_ALEN stride? If nothing else it seems like it would
be better to just create a structure or typedef containing the u8 array
and to return a pointer to that since all the ETH_ALEN here really
represents is a stride within your array.
> +{
> + bool is_uc = (list_type == MLX5_NVPRT_LIST_TYPE_UC);
> + struct net_device *ndev = priv->netdev;
> + struct mlx5e_eth_addr_hash_node *hn;
> + struct hlist_head *addr_list;
> + u8 (*addr_array)[ETH_ALEN];
> + struct hlist_node *tmp;
> + int max_list_size;
> + int list_size;
> + int hi;
> + int i;
> +
> + list_size = is_uc ? 0 : (priv->eth_addr.broadcast_enabled ? 1 : 0);
> + max_list_size = is_uc ?
> + 1 << MLX5_CAP_GEN(priv->mdev, log_max_current_uc_list) :
> + 1 << MLX5_CAP_GEN(priv->mdev, log_max_current_mc_list);
> +
> + addr_list = is_uc ? priv->eth_addr.netdev_uc : priv->eth_addr.netdev_mc;
> + mlx5e_for_each_hash_node(hn, tmp, addr_list, hi)
> + list_size++;
> +
> + if (list_size > max_list_size) {
> + netdev_warn(ndev,
> + "netdev %s list size (%d) > (%d) max vport list size, some addresses will be dropped\n",
> + is_uc ? "UC" : "MC", list_size, max_list_size);
> + list_size = max_list_size;
> + }
> +
> + addr_array = kcalloc(list_size, ETH_ALEN, GFP_KERNEL);
> + if (!addr_array)
> + return NULL;
> +
> + i = 0;
> + if (is_uc) { /* Make sure our own address is pushed first */
> + mlx5e_for_each_hash_node(hn, tmp, addr_list, hi) {
> + if (ether_addr_equal(ndev->dev_addr, hn->ai.addr)) {
> + ether_addr_copy(addr_array[i++], ndev->dev_addr);
> + break;
> + }
> + }
> + }
> +
What is the point of this loop? Is there a chance that the device
address isn't going to be in the list somewhere? Otherwise it seems
like you could just follow the pattern you did for the broadcast address
and just copy the dev_addr directly instead of crawling through the loop.
> + if (!is_uc && priv->eth_addr.broadcast_enabled)
> + ether_addr_copy(addr_array[i++], ndev->broadcast);
> +
> + mlx5e_for_each_hash_node(hn, tmp, addr_list, hi) {
> + if (ether_addr_equal(ndev->dev_addr, hn->ai.addr))
> + continue;
> + if (i >= list_size)
> + break;
> + ether_addr_copy(addr_array[i++], hn->ai.addr);
> + }
> +
> + *size = list_size;
> + return addr_array;
> +}
> +
> +static void mlx5e_vport_context_update_addr_list(struct mlx5e_priv *priv,
> + int list_type)
> +{
> + bool is_uc = (list_type == MLX5_NVPRT_LIST_TYPE_UC);
> + u8 (*mac_list)[ETH_ALEN];
> + int list_size;
> + int err;
> +
> + mac_list = mlx5e_build_addr_array(priv, list_type, &list_size);
> + if (!mac_list) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + err = mlx5_modify_nic_vport_mac_list(priv->mdev,
> + list_type,
> + mac_list,
> + list_size);
> +out:
> + if (err)
> + netdev_err(priv->netdev,
> + "Failed to modify vport %s list err(%d)\n",
> + is_uc ? "UC" : "MC", err);
> + kfree(mac_list);
> +}
> +
> +static void mlx5e_vport_context_update(struct mlx5e_priv *priv)
> +{
> + struct mlx5e_eth_addr_db *ea = &priv->eth_addr;
> +
> + mlx5e_vport_context_update_addr_list(priv, MLX5_NVPRT_LIST_TYPE_UC);
> + mlx5e_vport_context_update_addr_list(priv, MLX5_NVPRT_LIST_TYPE_MC);
> + mlx5_modify_nic_vport_promisc(priv->mdev, 0,
> + ea->allmulti_enabled,
> + ea->promisc_enabled);
> +}
> +
> static void mlx5e_apply_netdev_addr(struct mlx5e_priv *priv)
> {
> struct mlx5e_eth_addr_hash_node *hn;
> @@ -748,6 +845,8 @@ void mlx5e_set_rx_mode_work(struct work_struct *work)
> ea->promisc_enabled = promisc_enabled;
> ea->allmulti_enabled = allmulti_enabled;
> ea->broadcast_enabled = broadcast_enabled;
> +
> + mlx5e_vport_context_update(priv);
> }
>
> void mlx5e_init_eth_addr(struct mlx5e_priv *priv)
>
--
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