Re: [Patch] infiniband: check local reserved ports

Re: [Patch] infiniband: check local reserved ports

[Roland Dreier]

 > Since Tetsuo's patch already got merged, now this is the missing part
 > for local port reservation.
 > 
 > Cc: Roland Dreier <rdreier@cisco.com>
 > Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
 > Signed-off-by: WANG Cong <amwang@redhat.com>
 > 
 > ---
 > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
 > index b930b81..7b89bab 100644
 > --- a/drivers/infiniband/core/cma.c
 > +++ b/drivers/infiniband/core/cma.c
 > @@ -1978,6 +1978,7 @@ static int cma_alloc_any_port(struct idr *ps, struct rdma_id_private *id_priv)
 >  	rover = net_random() % remaining + low;
 >  retry:
 >  	if (last_used_port != rover &&
 > +	    !inet_is_reserved_local_port(rover) &&
 >  	    !idr_find(ps, (unsigned short) rover)) {
 >  		int ret = cma_alloc_port(ps, id_priv, rover);
 >  		/*

Should this inet_is_reserved_local_port() test apply to all the "port
spaces" that this code is handling?  I honestly am ignorant of the
intended semantics of the new local_reserved_ports stuff, hence my question.

 - R.
-- 
Roland Dreier <rolandd@cisco.com> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

Re: [Patch] infiniband: check local reserved ports

[Cong Wang]

On 06/04/10 00:39, Roland Dreier wrote:
>   >  Since Tetsuo's patch already got merged, now this is the missing part
>   >  for local port reservation.
>   >
>   >  Cc: Roland Dreier<rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>   >  Cc: Tetsuo Handa<penguin-kernel-1yMVhJb1mP/7nzcFbJAaVXf5DAMn2ifp@public.gmane.org>
>   >  Signed-off-by: WANG Cong<amwang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>   >
>   >  ---
>   >  diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>   >  index b930b81..7b89bab 100644
>   >  --- a/drivers/infiniband/core/cma.c
>   >  +++ b/drivers/infiniband/core/cma.c
>   >  @@ -1978,6 +1978,7 @@ static int cma_alloc_any_port(struct idr *ps, struct rdma_id_private *id_priv)
>   >   	rover = net_random() % remaining + low;
>   >   retry:
>   >   	if (last_used_port != rover&&
>   >  +	    !inet_is_reserved_local_port(rover)&&
>   >   	!idr_find(ps, (unsigned short) rover)) {
>   >   		int ret = cma_alloc_port(ps, id_priv, rover);
>   >   		/*
>
> Should this inet_is_reserved_local_port() test apply to all the "port
> spaces" that this code is handling?  I honestly am ignorant of the
> intended semantics of the new local_reserved_ports stuff, hence my question.
>

Yes, but I only found this case, is there any else?

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] infiniband: check local reserved ports

[Roland Dreier]

 > > Should this inet_is_reserved_local_port() test apply to all the "port
 > > spaces" that this code is handling?  I honestly am ignorant of the
 > > intended semantics of the new local_reserved_ports stuff, hence my question.

 > Yes, but I only found this case, is there any else?

My question was more in the other direction: should this test apply to
all the "port spaces" handled here?  From looking at the code, it
appears the answer is yes -- it seems that putting a port in
local_reserved_ports reserves that port for IPv4 and IPv6, UDP, TCP,
SCTP, DCCP, everything, so we should probably reserve all RDMA CM ports too.
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] infiniband: check local reserved ports

[Cong Wang]

On 06/05/10 00:04, Roland Dreier wrote:
>   >  >  Should this inet_is_reserved_local_port() test apply to all the "port
>   >  >  spaces" that this code is handling?  I honestly am ignorant of the
>   >  >  intended semantics of the new local_reserved_ports stuff, hence my question.
>
>   >  Yes, but I only found this case, is there any else?
>
> My question was more in the other direction: should this test apply to
> all the "port spaces" handled here?  From looking at the code, it
> appears the answer is yes -- it seems that putting a port in
> local_reserved_ports reserves that port for IPv4 and IPv6, UDP, TCP,
> SCTP, DCCP, everything, so we should probably reserve all RDMA CM ports too.

Yes.

So this patch looks good for you? :)

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] infiniband: check local reserved ports

[Roland Dreier]

 > So this patch looks good for you? :)

Yes, will queue it up, thanks.
-- 
Roland Dreier <rolandd@cisco.com> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

Re: [Patch] infiniband: check local reserved ports

[Cong Wang]

On 06/07/10 23:45, Roland Dreier wrote:
>   >  So this patch looks good for you? :)
>
> Yes, will queue it up, thanks.

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html