[ovs-dev] [PATCH v5 00/20] ovn-controller incremental processing

[Han Zhou]

On Fri, Sep 7, 2018 at 4:59 PM Han Zhou <zhouhan at gmail.com> wrote:
>
>
>
> On Fri, Sep 7, 2018 at 2:51 PM Ben Pfaff <blp at ovn.org> wrote:
> >
> > On Fri, Sep 07, 2018 at 02:50:34PM -0700, Ben Pfaff wrote:
> > > On Wed, Sep 05, 2018 at 05:11:01PM -0700, Han Zhou wrote:
> > > > On Wed, Sep 5, 2018 at 2:34 PM Ben Pfaff <blp at ovn.org> wrote:
> > > > >
> > > > > On Mon, Aug 13, 2018 at 10:47:59AM -0700, Han Zhou wrote:
> > > > > > ovn-controller currently recomputes everything when there are
any
> > > > changes
> > > > > > of input, which leads to high CPU usages and slow in end-to-end
flow
> > > > > > enforcement in response to changes. It even wastes CPU to
recompute
> > > > flows
> > > > > > for unrelated inputs such as pinctrl events.
> > > > >
> > > > > Would you mind publishing this as a branch I can pull from?
> > > >
> > > > Hi Ben,
> > > >
> > > > The branch for the series is here:
https://github.com/hzhou8/ovs/tree/ip12
> > >
> > > Thanks.  I am looking at it.
> > >
> > > I built the tip of the branch and saw only two warnings, from sparse:
> > >
> > >     ../ovn/controller/ovn-controller.c:616:12: error: symbol
'sb_engine_node_names' was not declared. Should it be static?
> > >     ../ovn/controller/ovn-controller.c:641:12: error: symbol
'ovs_engine_node_names' was not declared. Should it be static?
> > >
> > > Usually this warning means that the function or variable should be
> > > marked static.
> >
> > But in this case the variables can be deleted entirely:
> >
> > diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> > index 99e901663ae7..6f22ba66eabd 100644
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -613,12 +613,6 @@ enum sb_engine_node {
> >  #undef SB_NODE
> >  };
> >
> > -const char *sb_engine_node_names[] = {
> > -#define SB_NODE(NAME, NAME_STR) "SB_"NAME_STR,
> > -    SB_NODES
> > -#undef SB_NODE
> > -};
> > -
> >  #define SB_NODE_NAME(NAME) sb_engine_node_names[SB_##NAME]
> >
> >  #define SB_NODE(NAME, NAME_STR) ENGINE_FUNC_SB(NAME);
> > @@ -638,12 +632,6 @@ enum ovs_engine_node {
> >  #undef OVS_NODE
> >  };
> >
> > -const char *ovs_engine_node_names[] = {
> > -#define OVS_NODE(NAME, NAME_STR) "OVS_"NAME_STR,
> > -    OVS_NODES
> > -#undef OVS_NODE
> > -};
> > -
> >  #define OVS_NODE_NAME(NAME) ovs_engine_node_names[OVS_##NAME]
> >
> >  #define OVS_NODE(NAME, NAME_STR) ENGINE_FUNC_OVS(NAME);
>
> Hi Ben,
>
> Yes, I was using it in the beginning but refactored later and forgot to
remove them. I will remove them in next version.
> I should try sparse for warnings.
>
> Thanks,
> Han

Hi Ben,

I updated the branch https://github.com/hzhou8/ovs/tree/ip12. Apart from
removing the unused macros, there are 2 more fixes. The fixes are rebased
into the ip12 branch. I kept the fixes on top of the original ip12 branch
you reviewed in a new branch
https://github.com/hzhou8/ovs/tree/ip12_fixes_for_v5, so that you can see
the difference easily. Please take a look when you get time. I will rebase
on master and submit v6 after getting your feedback for the whole series.

Thanks,
Han