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 <[email protected]>
I'd remove the inner parentheses here:
> + } 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:
> + hmap_remove(&ofproto->ofport_usage, &usage->hmap_node);
> + free(usage);
Acked-by: Ben Pfaff <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
