| Message ID | 201004150229.o3F2T4dZ054768@www262.sakura.ne.jp |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >Randomize local port allocation in a way sctp_get_port_local() does. >Update rover at the end of loop since we're likely to pick a valid port >on the first try. > >Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reviewed-by: Sean Hefty <sean.hefty@intel.com> >--- I like this version, thanks! I'm not sure which tree to merge it through. Are you needing this for 2.6.34, or is 2.6.35 okay? > drivers/infiniband/core/cma.c | 70 +++++++++++++++-------------------------- >- > 1 file changed, 25 insertions(+), 45 deletions(-) > >--- linux-2.6.34-rc4.orig/drivers/infiniband/core/cma.c >+++ linux-2.6.34-rc4/drivers/infiniband/core/cma.c >@@ -79,7 +79,6 @@ static DEFINE_IDR(sdp_ps); > static DEFINE_IDR(tcp_ps); > static DEFINE_IDR(udp_ps); > static DEFINE_IDR(ipoib_ps); >-static int next_port; > > struct cma_device { > struct list_head list; >@@ -1970,47 +1969,33 @@ err1: > > static int cma_alloc_any_port(struct idr *ps, struct rdma_id_private *id_priv) > { >- struct rdma_bind_list *bind_list; >- int port, ret, low, high; >- >- bind_list = kzalloc(sizeof *bind_list, GFP_KERNEL); >- if (!bind_list) >- return -ENOMEM; >- >-retry: >- /* FIXME: add proper port randomization per like inet_csk_get_port */ >- do { >- ret = idr_get_new_above(ps, bind_list, next_port, &port); >- } while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL)); >- >- if (ret) >- goto err1; >+ static unsigned int last_used_port; >+ int low, high, remaining; >+ unsigned int rover; > > inet_get_local_port_range(&low, &high); >- if (port > high) { >- if (next_port != low) { >- idr_remove(ps, port); >- next_port = low; >- goto retry; >- } >- ret = -EADDRNOTAVAIL; >- goto err2; >+ remaining = (high - low) + 1; >+ rover = net_random() % remaining + low; >+retry: >+ if (last_used_port != rover && >+ !idr_find(ps, (unsigned short) rover)) { >+ int ret = cma_alloc_port(ps, id_priv, rover); >+ /* >+ * Remember previously used port number in order to avoid >+ * re-using same port immediately after it is closed. >+ */ >+ if (!ret) >+ last_used_port = rover; >+ if (ret != -EADDRNOTAVAIL) >+ return ret; > } >- >- if (port == high) >- next_port = low; >- else >- next_port = port + 1; >- >- bind_list->ps = ps; >- bind_list->port = (unsigned short) port; >- cma_bind_port(bind_list, id_priv); >- return 0; >-err2: >- idr_remove(ps, port); >-err1: >- kfree(bind_list); >- return ret; >+ if (--remaining) { >+ rover++; >+ if ((rover < low) || (rover > high)) >+ rover = low; >+ goto retry; >+ } >+ return -EADDRNOTAVAIL; > } > > static int cma_use_port(struct idr *ps, struct rdma_id_private *id_priv) >@@ -2995,12 +2980,7 @@ static void cma_remove_one(struct ib_dev > > static int __init cma_init(void) > { >- int ret, low, high, remaining; >- >- get_random_bytes(&next_port, sizeof next_port); >- inet_get_local_port_range(&low, &high); >- remaining = (high - low) + 1; >- next_port = ((unsigned int) next_port % remaining) + low; >+ int ret; > > cma_wq = create_singlethread_workqueue("rdma_cm"); > if (!cma_wq) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sean Hefty wrote: > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > >> Randomize local port allocation in a way sctp_get_port_local() does. >> Update rover at the end of loop since we're likely to pick a valid port >> on the first try. >> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Reviewed-by: Sean Hefty <sean.hefty@intel.com> > Thanks, everyone! > > I like this version, thanks! I'm not sure which tree to merge it through. > Are you needing this for 2.6.34, or is 2.6.35 okay? > As soon as possible, so 2.6.34. :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Cong Wang wrote: > Sean Hefty wrote: > > I like this version, thanks! I'm not sure which tree to merge it through. > > Are you needing this for 2.6.34, or is 2.6.35 okay? > > > > As soon as possible, so 2.6.34. :) > Cong, merge window for 2.6.34 was already closed. You need to make your patchset towards 2.6.35 (using net-next-2.6 tree) rather than 2.6.34 (using linux-2.6 tree). Therefore, this patch being queued for 2.6.35 (through net-next-2.6 tree) should be okay for you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Fri, 16 Apr 2010 22:54:22 +0900 > Cong Wang wrote: >> Sean Hefty wrote: >> > I like this version, thanks! I'm not sure which tree to merge it through. >> > Are you needing this for 2.6.34, or is 2.6.35 okay? >> > >> >> As soon as possible, so 2.6.34. :) >> > Cong, merge window for 2.6.34 was already closed. > You need to make your patchset towards 2.6.35 (using net-next-2.6 tree) > rather than 2.6.34 (using linux-2.6 tree). Therefore, this patch being > queued for 2.6.35 (through net-next-2.6 tree) should be okay for you. I don't take RDMA patches into net-next-2.6, the less I touch this stack avoiding stuff the better and Roland has been taking this stuff into his own tree for some time now. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller wrote: > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Fri, 16 Apr 2010 22:54:22 +0900 > >> Cong Wang wrote: >>> Sean Hefty wrote: >>>> I like this version, thanks! I'm not sure which tree to merge it through. >>>> Are you needing this for 2.6.34, or is 2.6.35 okay? >>>> >>> As soon as possible, so 2.6.34. :) >>> >> Cong, merge window for 2.6.34 was already closed. >> You need to make your patchset towards 2.6.35 (using net-next-2.6 tree) >> rather than 2.6.34 (using linux-2.6 tree). Therefore, this patch being >> queued for 2.6.35 (through net-next-2.6 tree) should be okay for you. > > I don't take RDMA patches into net-next-2.6, the less I touch this > stack avoiding stuff the better and Roland has been taking this stuff > into his own tree for some time now. I left for a few days. Ok, so I will wait for this to be merged. Thanks, David and Tetsuo! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks, applied this part of the patch -- I preferred this one since the goto into the middle of a loop seemed worse than a goto out of the loop...
> Thanks, applied this part of the patch -- I preferred this one since the
err, not "part of the patch" -- I meant "this version of the patch".
--- linux-2.6.34-rc4.orig/drivers/infiniband/core/cma.c +++ linux-2.6.34-rc4/drivers/infiniband/core/cma.c @@ -79,7 +79,6 @@ static DEFINE_IDR(sdp_ps); static DEFINE_IDR(tcp_ps); static DEFINE_IDR(udp_ps); static DEFINE_IDR(ipoib_ps); -static int next_port; struct cma_device { struct list_head list; @@ -1970,47 +1969,33 @@ err1: static int cma_alloc_any_port(struct idr *ps, struct rdma_id_private *id_priv) { - struct rdma_bind_list *bind_list; - int port, ret, low, high; - - bind_list = kzalloc(sizeof *bind_list, GFP_KERNEL); - if (!bind_list) - return -ENOMEM; - -retry: - /* FIXME: add proper port randomization per like inet_csk_get_port */ - do { - ret = idr_get_new_above(ps, bind_list, next_port, &port); - } while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL)); - - if (ret) - goto err1; + static unsigned int last_used_port; + int low, high, remaining; + unsigned int rover; inet_get_local_port_range(&low, &high); - if (port > high) { - if (next_port != low) { - idr_remove(ps, port); - next_port = low; - goto retry; - } - ret = -EADDRNOTAVAIL; - goto err2; + remaining = (high - low) + 1; + rover = net_random() % remaining + low; +retry: + if (last_used_port != rover && + !idr_find(ps, (unsigned short) rover)) { + int ret = cma_alloc_port(ps, id_priv, rover); + /* + * Remember previously used port number in order to avoid + * re-using same port immediately after it is closed. + */ + if (!ret) + last_used_port = rover; + if (ret != -EADDRNOTAVAIL) + return ret; } - - if (port == high) - next_port = low; - else - next_port = port + 1; - - bind_list->ps = ps; - bind_list->port = (unsigned short) port; - cma_bind_port(bind_list, id_priv); - return 0; -err2: - idr_remove(ps, port); -err1: - kfree(bind_list); - return ret; + if (--remaining) { + rover++; + if ((rover < low) || (rover > high)) + rover = low; + goto retry; + } + return -EADDRNOTAVAIL; } static int cma_use_port(struct idr *ps, struct rdma_id_private *id_priv) @@ -2995,12 +2980,7 @@ static void cma_remove_one(struct ib_dev static int __init cma_init(void) { - int ret, low, high, remaining; - - get_random_bytes(&next_port, sizeof next_port); - inet_get_local_port_range(&low, &high); - remaining = (high - low) + 1; - next_port = ((unsigned int) next_port % remaining) + low; + int ret; cma_wq = create_singlethread_workqueue("rdma_cm"); if (!cma_wq)