lkml.org 
[lkml]   [2010]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [net-next PATCH v4 3/3] net: reserve ports for applications using fixed port numbers
    Octavian Purdila wrote:
    > This patch introduces /proc/sys/net/ipv4/ip_local_reserved_ports
    > (bitmap type) which allows users to reserve ports for third-party
    > applications.
    >
    > The reserved ports will not be used by automatic port assignments
    > (e.g. when calling connect() or bind() with port number 0). Explicit
    > port allocation behavior is unchanged.
    >
    > Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
    > Signed-off-by: WANG Cong <amwang@redhat.com>
    > Cc: Neil Horman <nhorman@tuxdriver.com>
    > Cc: Eric Dumazet <eric.dumazet@gmail.com>
    > ---
    > Documentation/networking/ip-sysctl.txt | 12 ++++++++++++
    > drivers/infiniband/core/cma.c | 7 ++++++-
    > include/net/ip.h | 6 ++++++
    > net/ipv4/af_inet.c | 8 +++++++-
    > net/ipv4/inet_connection_sock.c | 6 ++++++
    > net/ipv4/inet_hashtables.c | 2 ++
    > net/ipv4/sysctl_net_ipv4.c | 17 +++++++++++++++++
    > net/ipv4/udp.c | 3 ++-
    > net/sctp/socket.c | 2 ++
    > 9 files changed, 60 insertions(+), 3 deletions(-)
    >
    > diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
    > index 2dc7a1d..23be7a4 100644
    > --- a/Documentation/networking/ip-sysctl.txt
    > +++ b/Documentation/networking/ip-sysctl.txt
    > @@ -564,6 +564,18 @@ ip_local_port_range - 2 INTEGERS
    > (i.e. by default) range 1024-4999 is enough to issue up to
    > 2000 connections per second to systems supporting timestamps.
    >
    > +ip_local_reserved_ports - BITMAP of 65536 ports
    > + Specify the ports which are reserved for known third-party
    > + applications. These ports will not be used by automatic port assignments
    > + (e.g. when calling connect() or bind() with port number 0). Explicit
    > + port allocation behavior is unchanged.
    > +
    > + Reserving ports is done by writing positive numbers in this proc entry,
    > + clearing them is done by writing negative numbers (e.g. 8080 reserves
    > + port number, -8080 makes it available for automatic assignment again).
    > +
    > + Default: Empty
    > +
    > ip_nonlocal_bind - BOOLEAN
    > If set, allows processes to bind() to non-local IP addresses,
    > which can be quite useful - but may break some applications.
    > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
    > index cc9b594..8248fc6 100644
    > --- a/drivers/infiniband/core/cma.c
    > +++ b/drivers/infiniband/core/cma.c
    > @@ -1979,6 +1979,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;
    > } while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL));
    >
    > if (ret)
    > @@ -2997,10 +2999,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;
    >
    > cma_wq = create_singlethread_workqueue("rdma_cm");
    > if (!cma_wq)
    > diff --git a/include/net/ip.h b/include/net/ip.h
    > index fb63371..2e24256 100644
    > --- a/include/net/ip.h
    > +++ b/include/net/ip.h
    > @@ -184,6 +184,12 @@ extern struct local_ports {
    > } sysctl_local_ports;
    > extern void inet_get_local_port_range(int *low, int *high);
    >
    > +extern unsigned long *sysctl_local_reserved_ports;
    > +static inline int inet_is_reserved_local_port(int port)
    > +{
    > + return test_bit(port, sysctl_local_reserved_ports);
    > +}
    > +
    > extern int sysctl_ip_default_ttl;
    > extern int sysctl_ip_nonlocal_bind;
    >
    > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
    > index 7d12c6a..06810b0 100644
    > --- a/net/ipv4/af_inet.c
    > +++ b/net/ipv4/af_inet.c
    > @@ -1546,9 +1546,13 @@ static int __init inet_init(void)
    >
    > BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
    >
    > + sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
    > + if (!sysctl_local_reserved_ports)
    > + goto out;
    > +


    I think we should also consider the ports in ip_local_port_range,
    since we can only reserve the ports in that range.


    > rc = proto_register(&tcp_prot, 1);
    > if (rc)
    > - goto out;
    > + goto out_free_reserved_ports;
    >
    > rc = proto_register(&udp_prot, 1);
    > if (rc)
    > @@ -1647,6 +1651,8 @@ out_unregister_udp_proto:
    > proto_unregister(&udp_prot);
    > out_unregister_tcp_proto:
    > proto_unregister(&tcp_prot);
    > +out_free_reserved_ports:
    > + kfree(sysctl_local_reserved_ports);
    > goto out;
    > }
    >
    > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
    > index 8da6429..1acb462 100644
    > --- a/net/ipv4/inet_connection_sock.c
    > +++ b/net/ipv4/inet_connection_sock.c
    > @@ -37,6 +37,9 @@ struct local_ports sysctl_local_ports __read_mostly = {
    > .range = { 32768, 61000 },
    > };
    >
    > +unsigned long *sysctl_local_reserved_ports;
    > +EXPORT_SYMBOL(sysctl_local_reserved_ports);
    > +
    > void inet_get_local_port_range(int *low, int *high)
    > {
    > unsigned seq;
    > @@ -108,6 +111,8 @@ again:
    >
    > smallest_size = -1;
    > do {
    > + if (inet_is_reserved_local_port(rover))
    > + goto next_nolock;
    > head = &hashinfo->bhash[inet_bhashfn(net, rover,
    > hashinfo->bhash_size)];
    > spin_lock(&head->lock);
    > @@ -130,6 +135,7 @@ again:
    > break;
    > next:
    > spin_unlock(&head->lock);
    > + next_nolock:
    > if (++rover > high)
    > rover = low;
    > } while (--remaining > 0);
    > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
    > index 2b79377..d3e160a 100644
    > --- a/net/ipv4/inet_hashtables.c
    > +++ b/net/ipv4/inet_hashtables.c
    > @@ -456,6 +456,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
    > local_bh_disable();
    > for (i = 1; i <= remaining; i++) {
    > port = low + (i + offset) % remaining;
    > + if (inet_is_reserved_local_port(port))
    > + continue;
    > head = &hinfo->bhash[inet_bhashfn(net, port,
    > hinfo->bhash_size)];
    > spin_lock(&head->lock);
    > diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
    > index 7e3712c..ce3597a 100644
    > --- a/net/ipv4/sysctl_net_ipv4.c
    > +++ b/net/ipv4/sysctl_net_ipv4.c
    > @@ -298,6 +298,13 @@ static struct ctl_table ipv4_table[] = {
    > .mode = 0644,
    > .proc_handler = ipv4_local_port_range,
    > },
    > + {
    > + .procname = "ip_local_reserved_ports",
    > + .data = NULL, /* initialized in sysctl_ipv4_init */
    > + .maxlen = 65536,
    > + .mode = 0644,
    > + .proc_handler = proc_dobitmap,
    > + },


    Isn't there an off-by-one here?

    In patch 2/3, you use 0 to set the fist bit, then how about 65535 which
    writes 65536th bit? This is beyond the range of port number.


    > #ifdef CONFIG_IP_MULTICAST
    > {
    > .procname = "igmp_max_memberships",
    > @@ -721,6 +728,16 @@ static __net_initdata struct pernet_operations ipv4_sysctl_ops = {
    > static __init int sysctl_ipv4_init(void)
    > {
    > struct ctl_table_header *hdr;
    > + struct ctl_table *i;
    > +
    > + for (i = ipv4_table; i->procname; i++) {
    > + if (strcmp(i->procname, "ip_local_reserved_ports") == 0) {
    > + i->data = sysctl_local_reserved_ports;
    > + break;
    > + }
    > + }
    > + if (!i->procname[0])
    > + return -EINVAL;
    >
    > hdr = register_sysctl_paths(net_ipv4_ctl_path, ipv4_table);
    > if (hdr == NULL)
    > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
    > index 608a544..bfd0a6a 100644
    > --- a/net/ipv4/udp.c
    > +++ b/net/ipv4/udp.c
    > @@ -232,7 +232,8 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
    > */
    > do {
    > if (low <= snum && snum <= high &&
    > - !test_bit(snum >> udptable->log, bitmap))
    > + !test_bit(snum >> udptable->log, bitmap) &&
    > + !inet_is_reserved_local_port(snum))
    > goto found;
    > snum += rand;
    > } while (snum != first);
    > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
    > index f6d1e59..1f839d0 100644
    > --- a/net/sctp/socket.c
    > +++ b/net/sctp/socket.c
    > @@ -5432,6 +5432,8 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
    > rover++;
    > if ((rover < low) || (rover > high))
    > rover = low;
    > + if (inet_is_reserved_local_port(rover))
    > + continue;
    > index = sctp_phashfn(rover);
    > head = &sctp_port_hashtable[index];
    > sctp_spin_lock(&head->lock);



    \
     
     \ /
      Last update: 2010-02-16 10:37    [W:0.042 / U:0.512 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site