{"push_id":2589355058,"size":1,"distinct_size":1,"ref":"refs/heads/master","head":"476d2551abd2871696a64203f78d658ac2d7f32c","before":"3ab8a26e778e601f896f6a235d899a4aed754291","commits":[{"sha":"476d2551abd2871696a64203f78d658ac2d7f32c","author":{"name":"Ben Pfaff","email":"6f50513b21952446fbc8c95a8f361b7aa434b7c7@ovn.org"},"message":"rconn: Introduce new invariant to fix assertion failure in corner case.\n\nUntil now, rconn_get_version() has only reported the OpenFlow version in\nuse when the rconn is actually connected.  This makes sense, but it has a\nharsh consequence.  Consider code like this:\n\n    if (rconn_is_connected(rconn) && rconn_get_version(rconn) >= 0) {\n        for (int i = 0; i < 2; i++) {\n            struct ofpbuf *b = ofputil_encode_echo_request(\n                rconn_get_version(rconn));\n            rconn_send(rconn, b, NULL);\n        }\n    }\n\nMaybe not the smartest code in the world, and probably no one would write\nthis exact code in any case, but it doesn't look too risky or crazy.\n\nBut it is.  The second trip through the loop can assert-fail inside\nofputil_encode_echo_request() because rconn_get_version(rconn) returns -1\ninstead of a valid OpenFlow version.  That happens if the first call to\nrconn_send() encounters an error while sending the message and therefore\ndestroys the underlying vconn and disconnects so that rconn_get_version()\ndoesn't have a vconn to query for its version.\n\nIn a case like this where all the code to send the messages is close by, we\ncould just check rconn_get_version() in each loop iteration.  We could even\ngo through the tree and convince ourselves that individual bits of code are\nsafe, or be conservative and check rconn_get_version() >= 0 in the iffy\ncases.  But this seems to me like an ongoing source of risk and a way to\nget things wrong in corner cases.\n\nThis commit takes a different approach.  It introduces a new invariant: if\nan rconn has ever been connected, then it returns a valid OpenFlow version\nfrom rconn_get_version().  In addition, if an rconn is currently connected,\nthen the OpenFlow version it returns is the correct one (that may be\nobvious, but there were corner cases before where it returned -1 even\nthough rconn_is_connected() returned true).\n\nWith this commit, the code above would work OK.  If the first call to\nrconn_send() encounters an error sending the message, then\nrconn_get_version() in the second iteration will return the same value as\nin the first iteration.  The message passed to rconn_send() will end up\nbeing discarded, but that's much better than either an assertion failure or\nhaving to carefully analyze a lot of our code to deal with one unusual\ncorner case.\n\nReported-by: Han Zhou <zhouhan@gmail.com>\nSigned-off-by: Ben Pfaff <blp@ovn.org>\nAcked-by: Han Zhou <hzhou8@ebay.com>","distinct":true,"url":"https://api.github.com/repos/openvswitch/ovs/commits/476d2551abd2871696a64203f78d658ac2d7f32c"}]}
{"actor":{"display_login":"blp"}}