Re: [PATCH rdma-next 14/18] RDMA/mlx5: Process create QP flags in one place

[Leon Romanovsky <leon@kernel.org>]

On Fri, Apr 24, 2020 at 04:51:27PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 20, 2020 at 06:11:01PM +0300, Leon Romanovsky wrote:
> > +	process_create_flag(dev, &create_flags,
> > +			    IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK,
> > +			    MLX5_CAP_GEN(mdev, block_lb_mc), qp);
>
> This only applies to datagram QP types

We didn't really check it before, should I check it now?

>
> > +	process_create_flag(dev, &create_flags, IB_QP_CREATE_CROSS_CHANNEL,
> > +			    MLX5_CAP_GEN(mdev, cd), qp);
> > +	process_create_flag(dev, &create_flags, IB_QP_CREATE_MANAGED_SEND,
> > +			    MLX5_CAP_GEN(mdev, cd), qp);
> > +	process_create_flag(dev, &create_flags, IB_QP_CREATE_MANAGED_RECV,
> > +			    MLX5_CAP_GEN(mdev, cd), qp);
> > +
> > +	if (qp_type == IB_QPT_UD) {
> > +		process_create_flag(dev, &create_flags,
> > +				    IB_QP_CREATE_IPOIB_UD_LSO,
> > +				    MLX5_CAP_GEN(mdev, ipoib_basic_offloads),
> > +				    qp);
> > +		cond = MLX5_CAP_GEN(mdev, port_type) == MLX5_CAP_PORT_TYPE_IB;
> > +		process_create_flag(dev, &create_flags, IB_QP_CREATE_SOURCE_QPN,
> > +				    cond, qp);
> > +	}
> > +
> > +	if (qp_type == IB_QPT_RAW_PACKET) {
> > +		cond = MLX5_CAP_GEN(mdev, eth_net_offloads) &&
> > +		       MLX5_CAP_ETH(mdev, scatter_fcs);
> > +		process_create_flag(dev, &create_flags,
> > +				    IB_QP_CREATE_SCATTER_FCS, cond, qp);
> > +
> > +		cond = MLX5_CAP_GEN(mdev, eth_net_offloads) &&
> > +		       MLX5_CAP_ETH(mdev, vlan_cap);
> > +		process_create_flag(dev, &create_flags,
> > +				    IB_QP_CREATE_CVLAN_STRIPPING, cond, qp);
> > +	}
> > +
> > +	process_create_flag(dev, &create_flags,
> > +			    IB_QP_CREATE_PCI_WRITE_END_PADDING,
> > +			    MLX5_CAP_GEN(mdev, end_pad), qp);
>
> This one is datagram only toos

Same

>
> > +
> > +	process_create_flag(dev, &create_flags, MLX5_IB_QP_CREATE_WC_TEST,
> > +			    qp_type != MLX5_IB_QPT_REG_UMR, qp);
> > +	process_create_flag(dev, &create_flags, MLX5_IB_QP_CREATE_SQPN_QP1,
> > +			    true, qp);
>
> I wonder if these are excluded from userspace someplace, seems like it
> is worth a udata test here just to be clear

We are excluding them in create_qp():drivers/infiniband/core/uverbs_cmd.c

1411         if (attr.create_flags & ~(IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK |
1412                                 IB_QP_CREATE_CROSS_CHANNEL |
1413                                 IB_QP_CREATE_MANAGED_SEND |
1414                                 IB_QP_CREATE_MANAGED_RECV |
1415                                 IB_QP_CREATE_SCATTER_FCS |
1416                                 IB_QP_CREATE_CVLAN_STRIPPING |
1417                                 IB_QP_CREATE_SOURCE_QPN |
1418                                 IB_QP_CREATE_PCI_WRITE_END_PADDING)) {
1419                 ret = -EINVAL;
1420                 goto err_put;
1421         }

>
> > +
> > +	if (create_flags)
> > +		mlx5_ib_dbg(dev, "Create QP has unsupported flags 0x%X\n",
> > +			    create_flags);
> > +
> > +	return (create_flags) ? -EINVAL : 0;
>
> Since there is already an if, avoid ternary expression

No problem

>
> Jason