alloc_ofp_port() does not allocate the port number correctly if the port
number passed initially is already in use. The following if block
if (ofp_port >= ofproto->max_ports
|| bitmap_is_set(ofproto->ofp_port_ids, ofp_port)) {
is entered when either of the two conditions is true but the while block
after this is not entered if the second condition above is true and the
first condition is false.
This results in an existing port_number to be re-assigned!
Signed-off-by: Krishna Kondaka <kkondaka@vmware.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Jun Nakajima jun.nakajima@intel.com
Justin Pettit jpettit@nicira.com
Keith Amidon keith@nicira.com
Jun Nakajima jun.nakajima@intel.com
Justin Pettit jpettit@nicira.com
Keith Amidon keith@nicira.com
+Krishna Kondaka kkondaka@vmware.com
Kyle Mestery kmestery@cisco.com
Leo Alterman lalterman@nicira.com
Luca Giraudo lgiraudo@nicira.com
Kyle Mestery kmestery@cisco.com
Leo Alterman lalterman@nicira.com
Luca Giraudo lgiraudo@nicira.com
alloc_ofp_port(struct ofproto *ofproto, const char *netdev_name)
{
uint16_t ofp_port;
alloc_ofp_port(struct ofproto *ofproto, const char *netdev_name)
{
uint16_t ofp_port;
+ uint16_t end_port_no = ofproto->alloc_port_no;
ofp_port = simap_get(&ofproto->ofp_requests, netdev_name);
ofp_port = ofp_port ? ofp_port : OFPP_NONE;
if (ofp_port >= ofproto->max_ports
|| bitmap_is_set(ofproto->ofp_port_ids, ofp_port)) {
ofp_port = simap_get(&ofproto->ofp_requests, netdev_name);
ofp_port = ofp_port ? ofp_port : OFPP_NONE;
if (ofp_port >= ofproto->max_ports
|| bitmap_is_set(ofproto->ofp_port_ids, ofp_port)) {
- bool retry = ofproto->alloc_port_no ? true : false;
-
/* Search for a free OpenFlow port number. We try not to
* immediately reuse them to prevent problems due to old
* flows. */
/* Search for a free OpenFlow port number. We try not to
* immediately reuse them to prevent problems due to old
* flows. */
- while (ofp_port >= ofproto->max_ports) {
- for (ofproto->alloc_port_no++;
- ofproto->alloc_port_no < ofproto->max_ports;
- ofproto->alloc_port_no++) {
- if (!bitmap_is_set(ofproto->ofp_port_ids,
- ofproto->alloc_port_no)) {
- ofp_port = ofproto->alloc_port_no;
- break;
- }
+ for (;;) {
+ if (++ofproto->alloc_port_no >= ofproto->max_ports) {
+ ofproto->alloc_port_no = 0;
- if (ofproto->alloc_port_no >= ofproto->max_ports) {
- if (retry) {
- ofproto->alloc_port_no = 0;
- retry = false;
- } else {
- return OFPP_NONE;
- }
+ if (!bitmap_is_set(ofproto->ofp_port_ids,
+ ofproto->alloc_port_no)) {
+ ofp_port = ofproto->alloc_port_no;
+ break;
+ }
+ if (ofproto->alloc_port_no == end_port_no) {
+ return OFPP_NONE;
bitmap_set1(ofproto->ofp_port_ids, ofp_port);
return ofp_port;
}
bitmap_set1(ofproto->ofp_port_ids, ofp_port);
return ofp_port;
}