On 3 December 2015 at 18:39, Jarno Rajahalme <ja...@ovn.org> wrote: > > On Dec 3, 2015, at 6:19 PM, Joe Stringer <j...@ovn.org> wrote: > > On 3 December 2015 at 17:59, Jarno Rajahalme <ja...@ovn.org> wrote: > >> seq values are 64-bit, and storing them to a 32-bit variable causes >> the stored value never to match actual seq value after the seq value >> gets big enough. >> >> This is a likely cause of OVS main thread using 100% CPU in a system >> using bonds after some runtime. >> >> VMware-BZ: #1564993 >> Reported-by: Hiram Bayless <hbayl...@vmware.com> >> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> >> > > It looks like this bug goes back at least as far as v1.10, have we really > not hit it until now? > > Why aren't we getting any feedback from our compilers on this? > > It looks like there are several places with this kind of problem: > > $ grep -r unsigned.*change_seq > ./lib/netdev-windows.c: unsigned int change_seq; > ./lib/ovsdb-idl.c: unsigned int change_seqno; > ./lib/ovsdb-idl.c: unsigned int max_seqno = > table->change_seqno[OVSDB_IDL_CHANGE_INSERT]; > ./lib/ovsdb-idl-provider.h: unsigned int > change_seqno[OVSDB_IDL_CHANGE_MAX]; > ./lib/ovsdb-idl-provider.h: unsigned int > change_seqno[OVSDB_IDL_CHANGE_MAX]; > ./ofproto/bond.c: unsigned int change_seq; /* Tracks changes in > 'netdev'. */ > > > It is likely that in those other cases the value is not passed to > seq_wait(). >
Looks like you're right. As far as I can tell, the netdev-windows version doesn't do anything, and the ovsdb-idl instances out-date the existence of struct seq. The bond one on the other hand does actually use struct seq. So, other changes shouldn't be necessary. Acked-by: Joe Stringer <j...@ovn.org> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
