On Fri, Nov 08, 2013 at 09:41:20AM -0800, Joe Stringer wrote:
> Previously, we tracked status changes for ofports on a per-device basis.
> Each time in the main thread's loop, we would loop through all ofports
> and manually check whether the status has changed for corresponding
> devices.
> 
> This patch shifts change_seq above the netdevice layer, with one atomic
> variable tracking status change for all ports. In the average case where
> ports are not constantly going up or down, this allows us to check
> change_seq once per loop and not poll any ports. In the worst case,
> execution is expected to be similar to how it is currently.
> 
> As change_seq is no longer tracked per-device, it doesn't make sense to
> cache this status in each ofport struct. As such, we shift this into the
> ofproto struct.
> 
> In a test environment of 5000 internal ports and 50 tunnel ports with
> bfd, this reduces CPU usage from about 45% to about 35%.
> 
> Signed-off-by: Joe Stringer <[email protected]>

Good work, I really like changes that simplify code and speed it up.

I think it'd make sense to use a global 'struct seq' object instead of a
manually constructed sequence number.  Most notably that would allow
ofproto_wait() to avoid a race trying to wake up on any change.

I think that deleting the 'change_seq' member from 'struct netdev_class'
leaves no instructions in that file for how netdev class implementations
should report changes.  I think that we should add a comment to explain
that, probably as part of the top level comment on struct netdev_class.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to