[ovs-dev] [PATCH] ofproto-dpif: Allow setting of flow eviction threshold

[Ben Pfaff]

On Wed, Jul 27, 2011 at 05:39:05PM +0900, Simon Horman wrote:
> Allow setting the number of flows present in the flow hash
> at which point eviction of entries from the kernel flow hash
> will begin to occur.
> 
> The value may be set using a bridge's other-config column.
> 
> e.g.
> 
> ovs-vsctl set bridge br3 other-config:flow-eviction-threshold=10000
> 
> default is 1000, reflecting constant value previously used.
> 
> Increasing this value can result in reduced CPU usage and
> packet loss in situations where the number of active flows
> is significantly larger than 1000.

Thanks for passing this along.  It looks pretty good.  I have a few
comments.

The struct ofproto member could use a comment.  This affects only the
ofproto-dpif implementation, so it would be worth mentioning that.  (If
we get any more ofproto-dpif only features then we'll want to move them
out of the generic structure.)

I don't see anything that initializes the threshold in the absence of a
call to ofproto_set_flow_eviction_threshold().

I don't see anything that validates that a threshold that is set makes
sense.  I think that it would be appropriate for facet_max_idle() to
force a minimum of at least, say, 100.

There's a typo in vswitch.xml: s/fromt he/from the/.

The commit log does a good job of documenting this option, but I'd
recommend also mentioning in vswitch.xml the default, the minimum value,
the reason why one might want to change the default.  It's probably also
worth mentioning that this affects only the software-based switch.

Thanks,

Ben.