On Mon, Jan 05, 2015 at 01:03:59PM -0800, Jarno Rajahalme wrote:
> Looks good to me, with a small nit/comment below,
>
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
> Jarno
>
> On Jan 1, 2015, at 11:06 AM, Ben Pfaff <b...@nicira.com> wrote:
>
> > The hidden rules created by in-band control and fail-open should not be
> > included in the table stats reported via OpenFlow. I seem to recall that
> > this was done correctly in some previous version but it has broken since
> > then. This commit fixes the problem and adds a test that should make it
> > harder to break again in the future.
> >
> > Reported-by: Ashok Chippa <a.n.chi...@gmail.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > ofproto/connmgr.c | 19 ++++++++++++++++++-
> > ofproto/connmgr.h | 4 +++-
> > ofproto/fail-open.c | 13 ++++++++++++-
> > ofproto/fail-open.h | 4 +++-
> > ofproto/in-band.c | 10 +++++++++-
> > ofproto/in-band.h | 4 +++-
> > ofproto/ofproto.c | 6 +++++-
> > tests/ofproto.at | 52
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 105 insertions(+), 7 deletions(-)
> >
> > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> > index 08abe1e..3d69122 100644
> > --- a/ofproto/connmgr.c
> > +++ b/ofproto/connmgr.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -1980,6 +1980,23 @@ connmgr_flushed(struct connmgr *mgr)
> > ofpbuf_uninit(&ofpacts);
> > }
> > }
> > +
> > +/* Returns the number of hidden rules created by the in-band and fail-open
> > + * implementations in table 0. (Subtracting this count from the number of
> > + * rules in the table 0 classifier, as returned by classifier_count(),
> > yields
> > + * the number of flows that OVS should report via OpenFlow for table 0.) */
> > +int
> > +connmgr_count_hidden_rules(const struct connmgr *mgr)
> > +{
> > + int n_hidden = 0;
> > + if (mgr->in_band) {
> > + n_hidden += in_band_count_rules(mgr->in_band);
> > + }
> > + if (mgr->fail_open) {
> > + n_hidden += fail_open_count_rules(mgr->fail_open);
> > + }
> > + return n_hidden;
> > +}
> >
> > /* Creates a new ofservice for 'target' in 'mgr'. Returns 0 if successful,
> > * otherwise a positive errno value.
> > diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
> > index c0f7c35..dd1a027 100644
> > --- a/ofproto/connmgr.h
> > +++ b/ofproto/connmgr.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -193,6 +193,8 @@ bool connmgr_has_in_band(struct connmgr *);
> > /* Fail-open and in-band implementation. */
> > void connmgr_flushed(struct connmgr *);
> >
> > +int connmgr_count_hidden_rules(const struct connmgr *);
> > +
> > /* A flow monitor managed by NXST_FLOW_MONITOR and related requests. */
> > struct ofmonitor {
> > struct ofconn *ofconn; /* Owning 'ofconn'. */
> > diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
> > index ecdba44..d3faf10 100644
> > --- a/ofproto/fail-open.c
> > +++ b/ofproto/fail-open.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -76,6 +76,7 @@ struct fail_open {
> > int last_disconn_secs;
> > long long int next_bogus_packet_in;
> > struct rconn_packet_counter *bogus_packet_counter;
> > + bool fail_open_active;
> > };
> >
> > static void fail_open_recover(struct fail_open *);
> > @@ -234,6 +235,15 @@ fail_open_flushed(struct fail_open *fo)
> >
> > ofpbuf_uninit(&ofpacts);
> > }
> > + fo->fail_open_active = open;
> > +}
> > +
> > +/* Returns the number of fail-open rules currently installed in the flow
> > + * table. */
> > +int
> > +fail_open_count_rules(const struct fail_open *fo)
> > +{
> > + return fo->fail_open_active;
>
> I’m not sure this is in line with CodingStyle, but it may be that
> CodingStyle could be updated now that we compile with C99?
It took me a minute to see what you mean here. I could sit and
microanalyze it, but it seems easier to just add "!= 0". so that's what
I did.
Thanks! Applied to master.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
