[ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

Simon Horman simon.horman at netronome.com
Wed Mar 27 10:52:09 UTC 2019


On Wed, Mar 27, 2019 at 10:37:03AM +0100, Simon Horman wrote:
> On Tue, Mar 26, 2019 at 02:17:07PM +0100, Simon Horman wrote:
> > On Tue, Mar 26, 2019 at 04:50:03PM +0800, solomon wrote:
> > > Ilya Maximets wrote:
> > > >> On Thu, Mar 21, 2019 at 10:41:05AM +0800, solomon wrote:
> > > >>> Ben Pfaff wrote:
> > > >>>> On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:
> > > >>>>>
> > > >>>>> After inserting/removing a bucket, we don't update the bucket counter.
> > > >>>>> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
> > > >>>>
> > > >>>> Thanks for the patch!  It looks correct to me.  Thank you for adding a
> > > >>>> test, too.
> > > >>>>
> > > >>>> I took a closer look and I saw that 'n_buckets' is not very useful,
> > > >>>> because it is only used in cases where the code is already
> > > >>>> O(n_buckets).  I think that we can just remove it.  Then it cannot get
> > > >>>> out-of-sync.  What do you think of this variation of your patch?
> > > >>>
> > > >>>
> > > >>> ovs_list_size() will traversing the list to get the total length.
> > > >>>
> > > >>> In our custom scheduling algorithms (eg wrr, least-connection), 
> > > >>> we need to know the total number of buckets before traversing the bucket list to hit target bucket. 
> > > >>> so, it is traversed twice.
> > > >>>
> > > >>> If the number of buckets reaches 100+, there are tens of thousands of groups, don't this modification affect performance?
> > > >>>
> > > >>> I hope to keep n_buckets in struct ofgroup.
> > > >>
> > > >> OK.
> > > >>
> > > >> I applied the original to master and backported as far as branch-2.6.
> > > > 
> > > > This patch broke the testsuite on branches 2.6 to 2.10.
> > > 
> > > The new testcase in this patch insert new bucket using insert-buckets command.
> > > But there is a bug in inserting bucket with weight fixed in commit 
> > > 0b4caa2eba22a516a312e7b404107eaebe618ee1
> > > (ofp-group: support to insert bucket with weight value for select type)
> > > 
> > > Also need to backport commit 0b4caa2eba to branch2.6~2.10.
> > 
> > Thanks, I have made preliminary backports to the relevant branches
> > and am running travisci to see if the tests pass.
> > 
> > https://travis-ci.org/horms2/ovs/builds/511479533
> 
> The check above, of the backport to branch-2.10, succeeded so I have
> pushed the backport to the ovs tree.
> 
> > https://travis-ci.org/horms2/ovs/builds/511482871
> > https://travis-ci.org/horms2/ovs/builds/511482977
> > https://travis-ci.org/horms2/ovs/builds/511482911
> > https://travis-ci.org/horms2/ovs/builds/511482945
> 
> The above backports to branches 2.6 to 2.9 were incomplete.
> I have made a second attempt. Travis is checking over that:
> 
> https://travis-ci.org/horms2/ovs/builds/511479533
> https://travis-ci.org/horms2/ovs/builds/511897524
> https://travis-ci.org/horms2/ovs/builds/511897703
> https://travis-ci.org/horms2/ovs/builds/511897927

The above passed and I have now pushed the (complete) backport of
0b4caa2eba22 ("ofp-group: support to insert bucket with weight value for
select type") to branch-2.6 to 2.9.


More information about the dev mailing list