On Wed, Feb 15, 2012 at 11:31 AM, Pravin B Shelar <[email protected]> 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.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
