[ovs-dev] [Bug 2462] [PATCH v3] datapath: Increase maximum number of datapath ports.

[Jesse Gross]

On Wed, Feb 15, 2012 at 11:31 AM, Pravin B Shelar <pshelar at nicira.com> wrote:
> @@ -1754,24 +1790,21 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
>        if (!dp)
>                goto exit_unlock;
>
> +       if (dp->port_count >= DP_MAX_PORTS) {
> +               err = -EFBIG;
> +               goto exit_unlock;
> +       }

Now that we're limiting the maximum port number to 16 bit, I don't
think this check is as useful and in fact I don't see where we prevent
a user-specified port number from being above 64k.  Since we're
keeping the logic the same and only increasing the number of ports, I
don't think it's actually necessary to make any changes to
ovs_vport_cmd_new (and then we actually don't need dp->port_count
either because this is the only place we use it).

Also when you refer to the port number can use be sure to use a data
type that reflects its size like u16 and a name like 'port_no' that
reflects what it is.  I saw this primarily in ovs_lookup_vport() and
vport_hash_bucket().

> diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> index c06bb89..bc5e1ef 100644
> --- a/lib/dpif-linux.c
> +++ b/lib/dpif-linux.c
> @@ -400,7 +364,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
>     do {
>         uint32_t upcall_pid;
>
> -        request.port_no = dpif_linux_pop_port(dpif);
> +        request.port_no = ++dpif->alloc_port_no & MAX_PORTS;
>         upcall_pid = dpif_linux_port_get_pid(dpif_, request.port_no);
>         request.upcall_pid = &upcall_pid;
>         error = dpif_linux_vport_transact(&request, &reply, &buf);
> @@ -411,7 +375,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
>                      dpif_name(dpif_), request.port_no, upcall_pid);
>         }
>         ofpbuf_delete(buf);
> -    } while (request.port_no != UINT32_MAX
> +    } while ((i++ < MAX_PORTS)
>              && (error == EBUSY || error == EFBIG));

This is going to result in many system calls in the event that you are
running on a kernel that supports only 1024 ports as it loops through
the upper 63k port numbers.  You should expect that a userspace kernel
mismatch is a common case (as it almost certainly is when running with
an upstream kernel) and use the EFBIG return value as an indication of
when you've hit the limit.