Re: [PATCH for-next V5 10/12] IB/mlx4: Implement ib_device callbacks

Jason Gunthorpe
2015-06-11T06:31:08Z

On Mon, Jun 08, 2015 at 05:12:13PM +0300, Matan Barak wrote:

> +static struct net_device *mlx4_ib_get_netdev(struct ib_device *device, u8 port_num)
> +{

This function is never referenced in this patch, so we get compile
warnings?

Warnings are not a huge deal, but you did compile test every patch in
the series??

> + if (mlx4_is_bonded(ibdev->dev)) {
> + struct net_device *dev;
> + struct net_device *upper = NULL;

So, I see this code in mlx4 touching bonding, but I don't see similar
code in ocrdma..

If bonding is a general feature why is this bit in the driver? Should
the core code be doing this?

Does bonding work in ocrdma at the end of this series? I was expecting
it to..

> + gid_tbl = mailbox->buf;
> +
> + for (i = 0; i < MLX4_MAX_PORT_GIDS; ++i)
> + memcpy(&gid_tbl[i], &gids[i].gid, sizeof(union ib_gid));
> +
> + err = mlx4_cmd(dev, mailbox->dma,
> + MLX4_SET_PORT_GID_TABLE << 8 | port_num,
> + 1, MLX4_CMD_SET_PORT, MLX4_CMD_TIME_CLASS_B,
> + MLX4_CMD_WRAPPED);
> + if (mlx4_is_bonded(dev))
> + err += mlx4_cmd(dev, mailbox->dma,
> + MLX4_SET_PORT_GID_TABLE << 8 | 2,
> + 1, MLX4_CMD_SET_PORT, MLX4_CMD_TIME_CLASS_B,
> + MLX4_CMD_WRAPPED);

Again, wonder why the driver is sensitive to bonding, and ocrdma is not..

> @@ -477,11 +673,22 @@ out:
> static int iboe_query_gid(struct ib_device *ibdev, u8 port, int index,
> union ib_gid *gid)
> {
> - struct mlx4_ib_dev *dev = to_mdev(ibdev);
> + int ret;
>
> - *gid = dev->iboe.gid_table[port - 1][index];
> + if (!rdma_cap_roce_gid_table(ibdev, port)) {
> + struct mlx4_ib_dev *dev = to_mdev(ibdev);
>
> - return 0;
> + *gid = dev->iboe.gid_table[port - 1][index];
> + return 0;
> + }
> +
> + ret = ib_get_cached_gid(ibdev, port, index, gid);
> + if (ret == -EAGAIN) {
> + memcpy(gid, &zgid, sizeof(*gid));
> + return 0;
> + }
> +
> + return ret;
> }

Hum, is it Ok to change iboe_query_gid like this at this point in the
series? Should this be in patch 11?

Jason