[ovs-dev] [PATCH] ofproto: Re-use port numbers of ports that were deleted an hour ago.

Gurucharan Shetty shettyg at nicira.com
Wed Oct 16 16:49:59 UTC 2013


On Tue, Oct 15, 2013 at 4:41 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Mon, Oct 14, 2013 at 01:55:37PM -0700, Gurucharan Shetty wrote:
>> We have a least recently used algorithm for assigning ofport values
>> to newly created ports. i.e., when we don't have any unused ofport
>> numbers, we use ofport numbers from the oldest deleted port.
>> What this means is that after using ~65000 previously unused ofport
>> numbers, we will have to go through all of the possible ports
>> to see which one was least recently used. This will eventually
>> slow down ofport allocation.
>>
>> With this commit, any port that was deleted more than an hour ago is
>> considered never to have been used. So it's ofport number becomes
>> free to be used.
>>
>> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
>
> I'd remove the inner parentheses here:
Done.
>
>> +            } else if ( last_used_at < (time_msec() - 60*60*1000)) {
>> +                /* If the port with ofport 'ofproto->alloc_port_no' was deleted
>> +                 * more than an hour ago, consider it usable. */
>> +                ofport_remove_usage(ofproto,
>> +                    u16_to_ofp(ofproto->alloc_port_no));
>> +                port_idx = ofproto->alloc_port_no;
>> +                break;
>>              } else if (last_used_at < lru) {
>>                  lru = last_used_at;
>>                  lru_ofport = ofproto->alloc_port_no;
>> @@ -2254,6 +2262,21 @@ ofport_set_usage(struct ofproto *ofproto, ofp_port_t ofp_port,
>>                  hash_ofp_port(ofp_port));
>>  }
>>
>> +static void
>> +ofport_remove_usage(struct ofproto *ofproto, ofp_port_t ofp_port)
>> +{
>> +    struct ofport_usage *usage;
>> +    HMAP_FOR_EACH_IN_BUCKET (usage, hmap_node, hash_ofp_port(ofp_port),
>> +                             &ofproto->ofport_usage) {
>> +        if (usage->ofp_port == ofp_port) {
>> +            break;
>> +        }
>> +    }
>> +
>
> I'd consider moving these two statements inside the loop, just before
> the "break;".  Then a minor programming error won't cause a segfault
> or corrupt memory:
Done.
>> +    hmap_remove(&ofproto->ofport_usage, &usage->hmap_node);
>> +    free(usage);
>
> Acked-by: Ben Pfaff <blp at nicira.com>
Thank you.



More information about the dev mailing list