[openib-general] [PATCHv6 RFC] IPoIB CM Experimental support

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCHv6 RFC] IPoIB CM Experimental support
> 
> Looks pretty good, but one thing worries me:
> 
> Overall looks great, I'll merge it up.

Great, thanks!
Just to clarify: do you intend to fix up the comments below or do you prefer for
me to do it and repost? If the later, it's easy for me, but I won't have access
to the lab today so an updated patch won't be tested till tomorrow.

> A few quick questions: > +#ifdef CONFIG_IPV6
> 
> I think this really needs to be
> 
> #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> 
> but I'm not clear on what happens if IPoIB is built-in and IPv6 is
> built as a module, since then icmpv6_send() isn't available until the
> ipv6 module is loaded.  It seems ip_gre.c has the same problem, so
> I'll ask on netdev about this.

I see this just got answered.

> Also a few other minor things:
> 
>  > +#ifdef CONFIG_INFINIBAND_IPOIB_CM
>  > +struct ib_cm_id;
> 
> this #ifdef in ipoib.h is just guarding declarations; we might as well
> declare everything even if it's not used.

Yes. I wasn't sure which way you'd prefer it.

>  > +	rep.starting_psn = 0 /* FIXME */;
> 
> any reason not to just do:
> 
> 	rep.starting_psn = random32() & 0xffffff;
> 
> ?

Well, randomness is a resource after all, and since we don't have the additional
security provided by PSNs in IPoIB UD, it seemed we do not need it for
IPoIB CM either. So maybe the right thing is just to remove the FIXME comment.

>  > +	req.srq 	              = 15;
> 
> This just should be 1, right?

Of course. It's a 1-bit field.

-- 
MST