[ovs-dev] [PATCH] bond: Use correct type for slave's change_seq.

[Joe Stringer]

On 3 December 2015 at 18:39, Jarno Rajahalme <jarno at ovn.org> wrote:

>
> On Dec 3, 2015, at 6:19 PM, Joe Stringer <joe at ovn.org> wrote:
>
> On 3 December 2015 at 17:59, Jarno Rajahalme <jarno at 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 <hbayless at vmware.com>
>> Signed-off-by: Jarno Rajahalme <jarno at 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 <joe at ovn.org>