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

Eli Cohen eli at dev.mellanox.co.il
Tue Jan 15 08:55:03 PST 2008


On a second thought, we could add - in addition to the creation flags -
a special qp type, IB_QPT_UD_IPOIB for example, where each low level
driver could add special enhancements for the sake of achieving higher
performance. What do you thinks?

On Tue, 2008-01-15 at 15:13 +0200, Eli Cohen wrote:
> 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?
> 
> 
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general




More information about the general mailing list