[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