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

[Jack Morgenstein]

At this time, ALL kernel callers of rdma_create_qp either:
- memset the qp_init_attr structure to zero
- define it with an initializer (which places zero in all unspecified fields).

Therefore, currently there is no problem. 

We might put in a note that callers should always cause unused fields of the ib_qp_init_attr
structure to be zero.

- Jack
> -----Original Message-----
> From: Eli Cohen 
> Sent: Tuesday, January 15, 2008 3:13 PM
> To: Roland Dreier; Jack Morgenstein
> Cc: openfabrics
> Subject: Re: [PATCH] ib/core: Add creation flags to create QP
> 
> 
> 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?
> 
> 
>