netdev - Re: [PATCH V1 net-next 3/7] mlx4: In RoCE allow guests to have multiple GIDS

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]

Message-Id: <20140312.010052.1153760698053567291.davem@davemloft.net>
Date:	Wed, 12 Mar 2014 01:00:52 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	ogerlitz@...lanox.com
Cc:	netdev@...r.kernel.org, roland@...nel.org,
	jackm@....mellanox.co.il, amirv@...lanox.com
Subject: Re: [PATCH V1 net-next 3/7] mlx4: In RoCE allow guests to have
 multiple GIDS

From: Or Gerlitz <ogerlitz@...lanox.com>
Date: Tue, 11 Mar 2014 15:02:52 +0200

> +static int get_slave_base_gid_ix(struct mlx4_ib_dev *dev, int slave, int port)
> +{
> +	int gids;
> +	int vfs;
> +
> +	if (rdma_port_get_link_layer(&dev->ib_dev, port) == IB_LINK_LAYER_INFINIBAND)
> +		return slave;
> +
> +	gids = MLX4_ROCE_MAX_GIDS - MLX4_ROCE_PF_GIDS;
> +	vfs = dev->dev->num_vfs;
> +
> +	if (slave == 0)
> +		return 0;
> +	if (slave <= gids % vfs)
> +		return MLX4_ROCE_PF_GIDS + ((gids / vfs) + 1) * (slave - 1);
> +
> +	return MLX4_ROCE_PF_GIDS + (gids % vfs) + ((gids / vfs) * (slave - 1));
> +}
> +
> +static int get_real_sgid_index(struct mlx4_ib_dev *dev, int slave, int port,
> +			       struct ib_ah_attr *ah_attr)
> +{
> +	if (rdma_port_get_link_layer(&dev->ib_dev, port) == IB_LINK_LAYER_INFINIBAND) {
> +		ah_attr->grh.sgid_index = slave;
> +		return 0;
> +	}
> +	ah_attr->grh.sgid_index += get_slave_base_gid_ix(dev, slave, port);
> +	return 0;
> +}

I hate to be picky like this, but this second function could be improved.

The first function is fine, it's named "get_*" and it really does return
a value.

But the second one, it's also named "get_*" but it doesn't return the
stated value.  Instead, it assigns it into a structure.

The return value is always zero, so I suggest you make it return void and
adjust the name.  Perhaps "fill_in_real_sgid_index", since that states
explicitly that the value is "filled in" to the passed ah_attr.
--
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