lkml.org 
[lkml]   [2010]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [Patch 3/3] net: reserve ports for applications using fixed port numbers
    From
    Date
    Hello.

    Amerigo Wang wrote:
    > Index: linux-2.6/drivers/infiniband/core/cma.c
    > ===================================================================
    > --- linux-2.6.orig/drivers/infiniband/core/cma.c
    > +++ linux-2.6/drivers/infiniband/core/cma.c
    > @@ -1980,6 +1980,8 @@ retry:
    > /* FIXME: add proper port randomization per like inet_csk_get_port */
    > do {
    > ret = idr_get_new_above(ps, bind_list, next_port, &port);
    > + if (inet_is_reserved_local_port(port))
    > + ret = -EAGAIN;

    You should not overwrite ret with -EAGAIN when idr_get_new_above() returned
    -ENOSPC. I don't know about idr, thus I don't know whether

    if (!ret && inet_is_reserved_local_port(port))
    ret = -EAGAIN;

    is correct or not.

    > } while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL));
    >
    > if (ret)
    > @@ -2996,10 +2998,13 @@ 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);
    > +again:
    > + get_random_bytes(&next_port, sizeof next_port);
    > remaining = (high - low) + 1;
    > next_port = ((unsigned int) next_port % remaining) + low;
    > + if (inet_is_reserved_local_port(next_port))
    > + goto again;
    >

    You should not unconditionally "goto again;".
    If all ports were reserved, it will loop forever (CPU stalls).

    > cma_wq = create_singlethread_workqueue("rdma_cm");
    > if (!cma_wq)


    > Index: linux-2.6/net/sctp/socket.c
    > ===================================================================
    > --- linux-2.6.orig/net/sctp/socket.c
    > +++ linux-2.6/net/sctp/socket.c
    > @@ -5436,6 +5436,8 @@ static long sctp_get_port_local(struct s
    > rover++;
    > if ((rover < low) || (rover > high))
    > rover = low;
    > + if (inet_is_reserved_local_port(rover))
    > + continue;

    This one needs to be

    if (inet_is_reserved_local_port(rover))
    goto next_nolock;

    > index = sctp_phashfn(rover);
    > head = &sctp_port_hashtable[index];
    > sctp_spin_lock(&head->lock);

    next:
    sctp_spin_unlock(&head->lock);
    +next_nolock:
    } while (--remaining > 0);

    otherwise, it will loop forever if all ports were reserved.

    \
     
     \ /
      Last update: 2010-04-09 15:25    [W:3.134 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site