[ovs-dev] [PATCH] meta-flow.h: Remove circular dependency that crept into meta-flow.h via the ofputil_protocol enum. This one is defined in ofp-util.h which itself includes meta-flow.h. This change replaces the enum with uint32_t which is the consitent choice within this module.

Jarno Rajahalme jrajahalme at nicira.com
Sat Oct 10 00:38:18 UTC 2015


John,

For some reason that email did not get to my email, nor our patchwork (https://patchwork.ozlabs.org/project/openvswitch/list/), but I found it from the dev at openvswitch.org <mailto:dev at openvswitch.org> archive.

You did not include a Signed-off-by line, so, in the interest of time, I put you as Suggested-by and myself as Signed-off-by. I also reformatted the commit message, so that it is not all in the title line. I also added a comment on the return value to mf_set() definition.

The fix is now in OVS master, I also added you to AUTHORS file.

Thank you for your contribution,

  Jarno

> On Oct 9, 2015, at 4:44 PM, John Reumann <nofutznetworks at gmail.com> wrote:
> 
> Sent to ovs devs. Do you know what the turn-around time is? For the time being I am patching our build but I am wondering if I should build a more permanent fix in place, like a fork. 
> 
> thx...
> 
> On Mon, Oct 5, 2015 at 4:06 PM, Jarno Rajahalme <jrajahalme at nicira.com <mailto:jrajahalme at nicira.com>> wrote:
> John,
> 
> I thought that maybe forward declaration of the enum would be better choice, but apparently C99 forbids forward declarations of enums, so even when it works with both gcc and clang, we should use your solution.
> 
> Please send the patch to dev at openvswitch.org <mailto:dev at openvswitch.org> instead, but trim the commit message to a paragraph that just describes the change.
> 
> Incidentally, I’m seeing the same IPv6 related test failures you see, but I figured it is due to the net-next kernel I’m developing for right now. It appears that ping6 works, but TCP over IPv6 does not. “telnet -4 localhost” responds with connection refused, but “telnet -6 ::1” just hangs. Could you tell me if IPv6 works on your machine?
> 
>   Jarno
> 
> > On Oct 2, 2015, at 6:34 PM, John Reumann <nofutznetworks at gmail.com <mailto:nofutznetworks at gmail.com>> wrote:
> >
> > This is a pretty trivial change as it picks up what you have already fixed before.
> > ofp-util.h include meta-flow.h. Before this patch some change caused meta-flow.h
> > to use ofp_protocol which is defined in ofp-util.h after it includes meta-flow.h.
> > The long term fix would probably be to move this enum into a separate file. But
> > for now this little patch might stop they pain when pulling in the files with
> > a c++ compiler.
> >
> > Tested with make check.... Code change seems not to produce extra failures.
> > But there are existing failures. Python currently not looking too hot.
> >
> > The diff between modified and original comes back empty
> >
> > diff ../../ovs-orig-src/ovs/tests/failed-test-orig.log tests/failed-test-modified.log
> >
> > comes back empty
> >
> > List of failures both at head and with my change.
> >
> > ## ------------------------ ##
> > ## Summary of the failures. ##
> > ## ------------------------ ##
> > Failed tests:
> > openvswitch 2.4.90 test suite test groups:
> >
> > NUM: FILE-NAME:LINE     TEST-GROUP-NAME
> >      KEYWORDS
> >
> > 877: ofproto-dpif.at:5056 <http://ofproto-dpif.at:5056/> ofproto-dpif - sFlow packet sampling - IPv6 collector
> > 883: ofproto-dpif.at:5426 <http://ofproto-dpif.at:5426/> ofproto-dpif - NetFlow flow expiration - IPv6 collector
> > 885: ofproto-dpif.at:5510 <http://ofproto-dpif.at:5510/> ofproto-dpif - NetFlow active expiration - IPv6 collector
> > 1465: ovsdb-server.at:929 <http://ovsdb-server.at:929/> insert default row, query table
> >      ovsdb server positive tcp6
> > 1508: ovsdb-idl.at:97 <http://ovsdb-idl.at:97/>    simple idl, initially empty, no ops - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1512: ovsdb-idl.at:104 <http://ovsdb-idl.at:104/>   simple idl, initially empty, various ops - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1516: ovsdb-idl.at:182 <http://ovsdb-idl.at:182/>   simple idl, initially populated - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1520: ovsdb-idl.at:213 <http://ovsdb-idl.at:213/>   simple idl, writing via IDL - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1524: ovsdb-idl.at:244 <http://ovsdb-idl.at:244/>   simple idl, handling verification failure - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1528: ovsdb-idl.at:275 <http://ovsdb-idl.at:275/>   simple idl, increment operation - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1532: ovsdb-idl.at:287 <http://ovsdb-idl.at:287/>   simple idl, aborting - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1536: ovsdb-idl.at:301 <http://ovsdb-idl.at:301/>   simple idl, destroy without commit or abort - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1540: ovsdb-idl.at:315 <http://ovsdb-idl.at:315/>   self-linking idl, consistent ops - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1544: ovsdb-idl.at:359 <http://ovsdb-idl.at:359/>   self-linking idl, inconsistent ops - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1548: ovsdb-idl.at:407 <http://ovsdb-idl.at:407/>   self-linking idl, sets - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> > 1552: ovsdb-idl.at:457 <http://ovsdb-idl.at:457/>   external-linking idl, consistent ops - Python tcp6
> >      ovsdb server idl positive python with tcp6 socket
> >
> > Skipped tests:
> > openvswitch 2.4.90 test suite test groups:
> >
> > NUM: FILE-NAME:LINE     TEST-GROUP-NAME
> >      KEYWORDS
> >
> > 110: daemon.at:163 <http://daemon.at:163/>      daemon --service
> >      windows-service
> > 421: vconn.at:22 <http://vconn.at:22/>        ssl vconn - refuse connection
> > 422: vconn.at:22 <http://vconn.at:22/>        ssl vconn - accept then close
> > 423: vconn.at:22 <http://vconn.at:22/>        ssl vconn - read hello
> > 424: vconn.at:22 <http://vconn.at:22/>        ssl vconn - send plain hello
> > 425: vconn.at:22 <http://vconn.at:22/>        ssl vconn - send long hello
> > 426: vconn.at:22 <http://vconn.at:22/>        ssl vconn - send echo hello
> > 427: vconn.at:22 <http://vconn.at:22/>        ssl vconn - send short hello
> > 428: vconn.at:22 <http://vconn.at:22/>        ssl vconn - send invalid version hello
> > 1410: ovsdb-server.at:486 <http://ovsdb-server.at:486/> SSL db: implementation
> >      ovsdb server positive ssl $5
> > 1413: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert default row, query table
> >      ovsdb server positive ssl
> > 1414: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert row, query table
> >      ovsdb server positive ssl
> > 1415: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, query by value
> >      ovsdb server positive ssl
> > 1416: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, query by named-uuid
> >      ovsdb server positive ssl
> > 1417: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, update rows by value
> >      ovsdb server positive ssl
> > 1418: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, mutate rows
> >      ovsdb server positive ssl
> > 1419: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, delete by named-uuid
> >      ovsdb server positive ssl
> > 1420: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, delete rows by value
> >      ovsdb server positive ssl
> > 1421: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, delete by (non-matching) value
> >      ovsdb server positive ssl
> > 1422: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert rows, delete all
> >      ovsdb server positive ssl
> > 1423: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert row, query table, commit
> >      ovsdb server positive ssl
> > 1424: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert row, query table, commit durably
> >      ovsdb server positive ssl
> > 1425: ovsdb-server.at:807 <http://ovsdb-server.at:807/> equality wait with correct rows
> >      ovsdb server positive ssl
> > 1426: ovsdb-server.at:807 <http://ovsdb-server.at:807/> equality wait with extra row
> >      ovsdb server positive ssl
> > 1427: ovsdb-server.at:807 <http://ovsdb-server.at:807/> equality wait with missing row
> >      ovsdb server positive ssl
> > 1428: ovsdb-server.at:807 <http://ovsdb-server.at:807/> inequality wait with correct rows
> >      ovsdb server positive ssl
> > 1429: ovsdb-server.at:807 <http://ovsdb-server.at:807/> inequality wait with extra row
> >      ovsdb server positive ssl
> > 1430: ovsdb-server.at:807 <http://ovsdb-server.at:807/> inequality wait with missing row
> >      ovsdb server positive ssl
> > 1431: ovsdb-server.at:807 <http://ovsdb-server.at:807/> insert and update constraints
> >      ovsdb server positive ssl
> > 1432: ovsdb-server.at:807 <http://ovsdb-server.at:807/> index uniqueness checking
> >      ovsdb server positive ssl
> > 1433: ovsdb-server.at:807 <http://ovsdb-server.at:807/> referential integrity -- simple
> >      ovsdb server positive ssl
> > 1434: ovsdb-server.at:807 <http://ovsdb-server.at:807/> referential integrity -- mutual references
> >      ovsdb server positive ssl
> > 1435: ovsdb-server.at:807 <http://ovsdb-server.at:807/> weak references
> >      ovsdb server positive ssl
> > 1436: ovsdb-server.at:807 <http://ovsdb-server.at:807/> immutable columns
> >      ovsdb server positive ssl
> > 1437: ovsdb-server.at:807 <http://ovsdb-server.at:807/> garbage collection
> >      ovsdb server positive ssl
> > 1438: ovsdb-server.at:845 <http://ovsdb-server.at:845/> insert default row, query table
> >      ovsdb server positive ssl6
> > 1597: ovs-vsctl.at:1311 <http://ovs-vsctl.at:1311/>  bootstrap ca cert
> >      ovs-vsctl ssl
> > 1598: ovs-vsctl.at:1338 <http://ovs-vsctl.at:1338/>  peer ca cert
> >      ovs-vsctl ssl
> >
> >
> > ---
> > lib/meta-flow.c | 2 +-
> > lib/meta-flow.h | 8 ++++----
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> > index 224ba53..9462b61 100644
> > --- a/lib/meta-flow.c
> > +++ b/lib/meta-flow.c
> > @@ -1616,7 +1616,7 @@ mf_set_wild(const struct mf_field *mf, struct match *match, char **err_str)
> >  * If non-NULL, 'err_str' returns a malloc'ed string describing any errors
> >  * with the request or NULL if there is no error. The caller is reponsible
> >  * for freeing the string.*/
> > -enum ofputil_protocol
> > +uint32_t
> > mf_set(const struct mf_field *mf,
> >        const union mf_value *value, const union mf_value *mask,
> >        struct match *match, char **err_str)
> > diff --git a/lib/meta-flow.h b/lib/meta-flow.h
> > index 02272ef..822cd83 100644
> > --- a/lib/meta-flow.h
> > +++ b/lib/meta-flow.h
> > @@ -1861,10 +1861,10 @@ void mf_get(const struct mf_field *, const struct match *,
> >             union mf_value *value, union mf_value *mask);
> >
> > /* Returns the set of usable protocols. */
> > -enum ofputil_protocol mf_set(const struct mf_field *,
> > -                             const union mf_value *value,
> > -                             const union mf_value *mask,
> > -                             struct match *, char **err_str);
> > +uint32_t mf_set(const struct mf_field *,
> > +                const union mf_value *value,
> > +                const union mf_value *mask,
> > +             struct match *, char **err_str);
> >
> > void mf_set_wild(const struct mf_field *, struct match *, char **err_str);
> >
> 
> 




More information about the dev mailing list