[ovs-dev] [PATCH] alloc_ofp_port does not allocate the port number correctly

Justin Pettit jpettit at nicira.com
Fri Jan 11 02:36:17 UTC 2013


On Jan 9, 2013, at 4:10 PM, Krishna Kondaka <kkondaka at vmware.com> wrote:

> -        /* Search for a free OpenFlow port number.  We try not to
> -         * immediately reuse them to prevent problems due to old
> -         * flows. */

I think the comment about not reusing port numbers is still useful for explaining why we're going through some contortions.

> +        do {

According to the OVS coding style (described in the "CodingStyle" file), infinite loops are written as "for (;;)".

> +            if (++ofproto->alloc_port_no == ofproto->max_ports)
> +                ofproto->alloc_port_no = 0;

According to the OVS coding style, single statements (such as "if") are always followed by braces.

> +            if (ofproto->alloc_port_no == end_port_no)
> +                return OFPP_NONE;

Same here.

Also, we don't ever try "alloc_port_no" again, which could have become valid since it was used as a port number in a previous call to this function.  It may be worth checking it before giving up and return OFPP_NONE.

> +            if (!bitmap_is_set(ofproto->ofp_port_ids,
> +                               ofproto->alloc_port_no)) {
> +                ofp_port = ofproto->alloc_port_no;
> +                break;
>             }
> -        }
> +        } while (1);
>     }
> -
>     bitmap_set1(ofproto->ofp_port_ids, ofp_port);
>     return ofp_port;
> }

Thanks for catching this bug and supplying a patch.  If you agree with my suggestions, do you mind updating the patch?

--Justin





More information about the dev mailing list