2010-06-03 08:27:13

by Cong Wang

[permalink] [raw]
Subject: [Patch] infiniband: check local reserved ports


Since Tetsuo's patch already got merged, now this is the missing part
for local port reservation.

Cc: Roland Dreier <[email protected]>
Cc: Tetsuo Handa <[email protected]>
Signed-off-by: WANG Cong <[email protected]>

---
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);
/*


2010-06-03 16:39:25

by Roland Dreier

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

> Since Tetsuo's patch already got merged, now this is the missing part
> for local port reservation.
>
> Cc: Roland Dreier <[email protected]>
> Cc: Tetsuo Handa <[email protected]>
> Signed-off-by: WANG Cong <[email protected]>
>
> ---
> 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 <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

2010-06-04 01:49:37

by Cong Wang

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

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<[email protected]>
> > Cc: Tetsuo Handa<[email protected]>
> > Signed-off-by: WANG Cong<[email protected]>
> >
> > ---
> > 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!

2010-06-04 16:04:59

by Roland Dreier

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

> > 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 <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

2010-06-07 09:00:31

by Cong Wang

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

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.

2010-06-07 15:45:29

by Roland Dreier

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

> So this patch looks good for you? :)

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

2010-06-08 02:20:05

by Cong Wang

[permalink] [raw]
Subject: Re: [Patch] infiniband: check local reserved ports

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

Thanks!