[ofa-general] Re: [PATCH] ib/core: Add creation flags to create QP

[Eli Cohen]

On Mon, 2008-01-14 at 14:18 -0800, Roland Dreier wrote:
> > +enum qp_create_flags {
>  > +	QP_CREATE_LSO = 1 << 0,
>  > +};
>  > +
>  >  struct ib_qp_init_attr {
>  >  	void                  (*event_handler)(struct ib_event *, void *);
>  >  	void		       *qp_context;
>  > @@ -496,6 +500,7 @@ struct ib_qp_init_attr {
>  >  	enum ib_sig_type	sq_sig_type;
>  >  	enum ib_qp_type		qp_type;
>  >  	u8			port_num; /* special QP types only */
>  > +	enum qp_create_flags	create_flags;
>  >  };
> 
> Not sure if this approach is a good one... would it make sense to
> create a new QP type like IB_QPT_UD_LSO to handle LSO instead?  Are
> there other flags we're going to want to add too?
As Jack replied already, we do need this also for his XRC code. Not
shown in the patch is that the flags representation at the hw level is
different from the verbs (QP_CREATE_LSO at the verbs layer is
MLX4_QP_LSO ).
> 
> Also this patch doesn't make much sense without the rest of the LSO
> stuff really.  Finally, I think you need to audit all the places where
> struct ib_qp_init_attr is used to make sure the flags are set
> correctly; for example the uverbs_cmd.c create QP function seems like
> it would end up passing a random stack value into create_flags.

I missed that one. There is one more place that might be a problem and
that is rdma_create_qp which is an exported function which accepts
struct ib_qp_init_attr * as an argument. This means that we need to
either clear the create_flags field or require to the caller to put a
valid value. What do you think?